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

Upgrade Proxy and Multisig pallets to Benchmarking V2 #6018

Merged

Conversation

re-gius
Copy link
Contributor

@re-gius re-gius commented Oct 10, 2024

Sub-task of PR #5995

Integration

These changes should not require any integration effort.

@re-gius re-gius changed the base branch from master to kiz-frame-umbrella-in-pallets October 10, 2024 16:44
@re-gius re-gius added T2-pallets This PR/Issue is related to a particular pallet. T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed T2-pallets This PR/Issue is related to a particular pallet. labels Oct 10, 2024
@re-gius re-gius marked this pull request as ready for review October 11, 2024 07:35
@re-gius re-gius requested a review from a team as a code owner October 11, 2024 07:35
@re-gius re-gius self-assigned this Oct 11, 2024
@re-gius re-gius requested a review from kianenigma October 11, 2024 07:36
Copy link
Contributor

@seadanda seadanda left a comment

Choose a reason for hiding this comment

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

Looks like a few things need to be tidied up - make sure you're passing --features=runtime-benchmarks when you check

substrate/frame/proxy/src/benchmarking.rs Outdated Show resolved Hide resolved
substrate/frame/multisig/src/benchmarking.rs Outdated Show resolved Hide resolved
substrate/frame/proxy/src/benchmarking.rs Outdated Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team October 11, 2024 13:45
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

CI needs to pass, but looks good otherwise.

@github-actions github-actions bot requested a review from gui1117 October 14, 2024 09:03
Copy link

Review required! Latest push from author must always be reviewed

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Thank you!

Last question:

Please check the rust-doc of the each macro respectively, and see if the docs are sufficient and/or can be improved? Some ideas

  • Does the v1 one mention the existence of v2?
  • Is there a good "migration" guide available or noted anywhere?

If CI checks are not relevant, we can merge this anyways, as the target branch is not master.

@re-gius re-gius merged commit 253ce3f into kiz-frame-umbrella-in-pallets Oct 15, 2024
78 of 150 checks passed
@re-gius re-gius deleted the regius/migrate-benchmarking-pallets-pm branch October 15, 2024 15:32
github-merge-queue bot pushed a commit that referenced this pull request Oct 18, 2024
# Adding instruction to migrate benchmarking from v1 to v2

Even if the documentation for benchmarking v1 and v2 is clear and
detailed, I feel that adding a migration guide from v1 to v2 would help
doing it quicker.

## Integration
This change only affects documentation, so it does not cause integration
issues.

## Review Notes
I followed the migration procedure I applied in PR
#6018 . I added
everything from there, but I may be missing some extra steps that are
needed in specific case, so in case you notice something please let me
know.

---------

Co-authored-by: Dónal Murray <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants