-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow changing the GitHub API base URL via the INSTALLER_DOWNLOAD_URL env var #199
Conversation
566ea14
to
2de40fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This is looking good, and thanks so much for the tests.
Just one comment - the INSTALLER_DOWNLOAD_URL
environment variable means something very different to the installer. There it's the root URL at which the installer artifacts can be found, while here it's the GitHub API root. Those two URL structures aren't usually going to be compatible, so we'll want to pick a different name here - that way the two options can be set independently of each other and won't interfere.
Sorry if there was a mixup in the original issue over the choice of variable name! |
For my personal use case, that would be a disadvantage, not an advantage. Yes, these two might mean slightly different, but ultimately, from an end-user point of view, both of them are, hey, here are where the artifacts are hosted at. some artifacts are installer packages, other are github releases and api metadata are really just implementation details from an end user point of view. So I would suggest that you reconsider that. Having to set two separate environment variables to install and update my package would cause more confusion than how much benefit it would provide in my opinion. They might be beneficial for the one maintaining the site but would be a problem for the end users. |
@mistydemeo so how about a compromise of: use |
We use the INSTALLER_DOWNLOAD_URL to align up with the install script. Signed-off-by: Bernát Gábor <[email protected]>
Signed-off-by: Bernát Gábor <[email protected]>
Signed-off-by: Bernát Gábor <[email protected]>
Signed-off-by: Bernát Gábor <[email protected]>
👍 for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging this and our followup pull request. Thank you again very much for this!
Do you have a timeline when we should expect this feature to be available for uv? |
Resolves axodotdev/cargo-dist#1449