-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update nns.rs - use latest II and NNS Dapp #60
base: main
Are you sure you want to change the base?
Conversation
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.
Let's discuss whether we should instead update both of these to a specific more-recent version. Otherwise any future "latest" version of either of these dependencies will be able to break all past versions of the nns extension that depend on them.
Personally I would agree with linking a specific version. We have bots that periodically look for recent code, e.g. updates to dfx, the latest IC repo and so on and if an update is found, the bot opens a PR to update the version. We have CI tests that sometimes break when a version is updated so a human has to go in to make any changes related to the update. I believe that this gives a good balance of stability and having recent versions.
|
That said, the old URLs are really old, so this PR is an improvement over the status quo. If you have time to have a default version and override, I would recommend that and can help with it. If not, I would accept this PR. But this is not my call. |
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.
@krpeacock , would you please update this PR to use specific versions, and we will create follow-up tickets to create workflows to update them?
These might be the specific versions:
release-2023-08-28
for internet-identity (corresponds to proposal 124282, latest executed update to II canister)proposal-124328
for nns-dapp (proposal 124328 is latest executed update to nns-dapp canister, and corresponds toprod
tag today)
bump for @krpeacock |
As requested by Matthew Grogan, updates II and NNS Dapp for dfx extensions. This will automatically pull the wasm from the release tagged as latest