-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat: update axoupdater, fetch known-good version #1598
base: main
Are you sure you want to change the base?
Conversation
Instead of always fetching the latest axoupdater, fetch the version that this version of dist itself uses as a library. That way, we're always fetching something predictable.
const AXOUPDATER_MINIMUM_VERSION: &str = "0.7.0"; | ||
const AXOUPDATER_GIT_URL: &str = "https://github.com/axodotdev/axoupdater.git"; | ||
|
||
fn axoupdater_asset_root() -> String { | ||
format!("{AXOUPDATER_ASSET_ROOT}/v{}/download", axoupdater::VERSION) |
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.
does this prevent auto-updates?
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.
No, this is where we'll fetch standalone axoupdater builds from. It just means a given release of dist will always install a specific version of the standalone updater when packaging software, instead of a floating version. dist's own autoupdates will work.
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.
ah sorry i just realized how vague what i wrote was... i meant to ask if this means that a single version of dist will only package a single version of axoupdater and to use a new version of axo updater we'd need to make a new dist release
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.
Ah, got it. Yes, I felt that this was probably a good thing at this point - protection against breaking changes without a corresponding dist release, now that axoupdater is out of its initial experimental phase.
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.
This seems like a good improvement to me! As you mentioned in #1598 (comment), it seems prudent to avoid breaking changes happening where we don't expect them.
Instead of always fetching the latest axoupdater, fetch the version that this version of dist itself uses as a library. That way, we're always fetching something predictable.