Skip to content
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

Core proposals in upgrade handler #8844

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

mhofman
Copy link
Member

@mhofman mhofman commented Jan 30, 2024

closes: #8843

Description

Adds the ability to define core proposals as part of the upgrade handler.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

Some contributors documentation will be included in the release process

Testing Considerations

Upgrade Considerations

This change add new capabilities to our upgrade handler.

@mhofman mhofman added the force:integration Force integration tests to run on PR label Jan 30, 2024
@mhofman mhofman changed the title Mhofman/8843 core proposal upgrade handler Core proposals in upgrade handler Jan 30, 2024
@mhofman mhofman force-pushed the mhofman/8843-core-proposal-upgrade-handler branch from 9d52b1e to eb9ad28 Compare January 30, 2024 22:50
mhofman added a commit that referenced this pull request Jan 30, 2024
mhofman added a commit that referenced this pull request Jan 30, 2024
@mhofman mhofman marked this pull request as ready for review January 31, 2024 02:55
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few suggestions.

// Core proposals that should run during the upgrade block
// These will be merged with any coreProposals specified in the
// upgradeInfo field of the upgrade plan ran as subsequent steps
CoreProposals: vm.CoreProposalsFromSteps(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you supply a commented-out example of what real-world use would look like? I was confused by thinking that somehow vm.CoreProposalsFromSteps() was actually supposed to do something useful, rather than just generating an emptyish struct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have tests, but I recognize that may be infeasible due to time constraints.

@mhofman mhofman added the automerge:rebase Automatically rebase updates, then merge label Jan 31, 2024
@mhofman mhofman force-pushed the mhofman/8843-core-proposal-upgrade-handler branch from fa66528 to 0a4a43c Compare January 31, 2024 21:53
@mhofman mhofman force-pushed the mhofman/8843-core-proposal-upgrade-handler branch from 0a4a43c to 605eb4b Compare January 31, 2024 21:56
mhofman added a commit that referenced this pull request Jan 31, 2024
@mergify mergify bot merged commit a253f74 into master Jan 31, 2024
66 checks passed
@mergify mergify bot deleted the mhofman/8843-core-proposal-upgrade-handler branch January 31, 2024 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify core proposals in upgrade handler
2 participants