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

Disable MBM migrations for all runtimes for check-migrations CI #590

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Feb 12, 2025

Relates to: paritytech/try-runtime-cli#109

MBM migrations are not set up for any runtimes, but only Encointer has disabled checks for them.

  • Does not require a CHANGELOG entry

@bkontur
Copy link
Contributor Author

bkontur commented Feb 13, 2025

@ggwpez what do you suggest here? Wait for fix paritytech/try-runtime-cli#109 or merge this PR?

@bkchr
Copy link
Contributor

bkchr commented Feb 13, 2025

Why? Did we had any failing tests because of this?

@ggwpez
Copy link
Member

ggwpez commented Feb 13, 2025

@ggwpez what do you suggest here? Wait for fix paritytech/try-runtime-cli#109 or merge this PR?

I guess we have to merge this so that the tests run again properly. There is still no real maintainer of the try-runtime-cli, only a part-time one that has no availability until EOM.

@bkchr
Copy link
Contributor

bkchr commented Feb 14, 2025

/cmd bench --runtime coretime-kusama --pallet pallet_xcm

@bkchr
Copy link
Contributor

bkchr commented Feb 14, 2025

@bkontur just abusing your pr a little bit :P

Copy link

Command "bench --runtime coretime-kusama --pallet pallet_xcm" has started 🚀 See logs here

Copy link

Command "bench --runtime coretime-kusama --pallet pallet_xcm" has failed ❌! See logs here

@bkontur
Copy link
Contributor Author

bkontur commented Feb 14, 2025

Command "bench --runtime coretime-kusama --pallet pallet_xcm" has failed ❌! See logs here

@mordamax I think this subweight comparision should compare PR's branch against the PR's target/base branch here: https://github.com/polkadot-fellows/runtimes/blob/main/.github/workflows/cmd.yml#L351

because now it compares to the origin/main, which is in my fork, but it is not up-to-date - so tthat is why there is so big output for every pallet/runtime
https://github.com/polkadot-fellows/runtimes/actions/runs/13330241520/job/37234272415#step:15:10


So here: https://github.com/polkadot-fellows/runtimes/blob/main/.github/workflows/cmd.yml#L351 we should change refs/remotes/origin/main for something which points to the actual PR's target branch

@bkontur
Copy link
Contributor Author

bkontur commented Feb 14, 2025

/cmd bench --runtime coretime-kusama --pallet pallet_xcm

Copy link

Command "bench --runtime coretime-kusama --pallet pallet_xcm" has started 🚀 See logs here

Copy link

Command "bench --runtime coretime-kusama --pallet pallet_xcm" has finished ✅ See logs here

Subweight results:
File Extrinsic Old New Change [%]
system-parachains/coretime/coretime-kusama/src/weights/pallet_xcm.rs force_xcm_version 109.49us 116.36us +6.28
system-parachains/coretime/coretime-kusama/src/weights/pallet_xcm.rs take_response 163.85us 173.93us +6.15

@bkontur
Copy link
Contributor Author

bkontur commented Feb 14, 2025

@mordamax @bkchr very nice, looks like the bot finally passed :), but we need to fix this: #590 (comment)

@mordamax
Copy link
Contributor

@bkontur it definitely needs improvements, as it was first attempt here, and never worked to test&iterate. Feel free to change it :)

the polkadot-sdk one is a bit more advanced and stable

@bkontur
Copy link
Contributor Author

bkontur commented Feb 18, 2025

@bkontur it definitely needs improvements, as it was first attempt here, and never worked to test&iterate. Feel free to change it :)

the polkadot-sdk one is a bit more advanced and stable

@mordamax
This is as far as I can go for now: paritytech/polkadot-sdk#7602 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants