-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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 errors for additive and deponly policies #20695
Upgrade errors for additive and deponly policies #20695
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
4f6d491
to
137b942
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.
Some small questions here and there, but otherwise good to go!
...c/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__deponly.snap
Outdated
Show resolved
Hide resolved
...nit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__type_param.snap
Outdated
Show resolved
Hide resolved
impl InclusionCheckMode for CliInclusionCheckMode { | ||
type Error = Vec<UpgradeCompatibilityModeError>; | ||
|
||
fn module_name_mismatch(&mut self, _old_address: &Identifier, _new_address: &Identifier) {} |
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.
Is a module name mismatch even possible? I thought we performed this check by first lining up modules by their name. Should this case be removed from the interface?
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.
I think the move here is to bundle name and address check into a single module_id_mismatch
as compatibility check does so at least we don't have an extra function just for the name check. I'll do it in another smaller PR since it affects code outside of the sui crate.
137b942
to
ea81150
Compare
ea81150
to
5362a57
Compare
5362a57
to
d2a0931
Compare
d2a0931
to
1d359f2
Compare
Description
support additive and deponly policies for upgrade errors
Test plan
snapshot testing
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.