Skip to content

Conversation

@Melusion
Copy link

@Melusion Melusion commented Mar 18, 2025

regarding #2

Copy link
Owner

@gmta gmta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this!

README.md Outdated
3. Edit `.env` and set:
* `VELOCITA_URL`: the URL (e.g. `https://mydomain.tld`) on which Velocita is hosted
* `VELOCITA_IPV6_ENABLED`: set to `true` to enable IPv6
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to add this here; IPv6 being enabled is already the default. This section is meant to show some configuration options a lot of people might want to change when initially setting up the proxy.

velocita_url = velocita_url.sub(/\/+$/, '')
end
tls_enabled = ENV['VELOCITA_TLS_ENABLED'] == 'true'
ipv6_enabled = ENV['VELOCITA_IPV6_ENABLED'] == 'true'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Should be called VELOCITA_IPV6_LISTEN
  2. The default value should be true, so maybe make this (ENV['VELOCITA_IPV6_ENABLED'] || 'true') == 'true'?
  3. Could you document this new setting in proxy/README.md? Specifically this section: https://github.com/gmta/velocita-proxy/tree/master/proxy#configuring-velocita

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, will be
ipv6_listen = (ENV['VELOCITA_IPV6_LISTEN'] || 'true') == 'true'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely overlooked the second readme

.env.dist Outdated
VELOCITA_URL=http://localhost:8080
VELOCITA_HTTP_PORT=8080
VELOCITA_HTTPS_PORT=8443
VELOCITA_IPV6_ENABLED=true
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VELOCITA_IPV6_ENABLED=true
#VELOCITA_IPV6_LISTEN=true
  1. Let's name this "listen" instead of "enabled" since it does not affect IPv6 connectivity as a whole, but just on which interfaces nginx is listening.
  2. The default value should be true, so this is an optional setting.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check

@Melusion
Copy link
Author

Did I miss something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants