-
-
Notifications
You must be signed in to change notification settings - Fork 656
Upgrade external tools #22065
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
Upgrade external tools #22065
Conversation
c3b8cb9
to
5c591da
Compare
5c591da
to
1ffb917
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!
Co-authored-by: Huon Wilson <wilson.huon@gmail.com>
@huonw Thank you for review! Ready for merge
|
I believe it's the flaky test again |
default_version = "2.33.4" | ||
default_url_template = "https://github.com/pex-tool/pex/releases/download/v{version}/pex" |
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.
Thanks for thinking about aligning this to other tools... but I think this breaking change isn't appropriate in this form.
Can we (manually) switch back to the old style for this PR?
We can potentially make the breaking change as a separate piece of work, but we should do a deprecation for a while (i.e. support both old and new). In particular, I think it's relatively common for people to pin to a PEX version (particularly a newer one than the one in their current version of Pants) to pick up new features/bugfixes.
This change is instantly breaking anyone who does that, without any "live" explanation (i.e. when they try the upgrade, they'll just get download error explosions, rather than useful guidance about how to fix) and thus making their upgrade cycle harder.
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.
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.
Looks great now, except for some extra release notes. Thanks!
Co-authored-by: Huon Wilson <wilson.huon@gmail.com>
@huonw Done |
Thank you! |
The pr was generated with this command:
(Notes are not generated yet)
The tool code is here #22064