Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Co #14338: pallet-glutton: over-unity consumption #2730

Merged
merged 8 commits into from
Jun 13, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Jun 13, 2023

Companion for paritytech/substrate#14338

Change:

  • Perbill to FixedU64
  • Rename *_percent to *_ratio
  • Fix typos

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added B0-silent Changes should not be mentioned in any release notes A0-please_review Pull request needs code review. A4-companion A PR that should be considered alongside another (usually more comprehensive and detailed) PR. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jun 13, 2023
@ggwpez ggwpez requested a review from NachoPal June 13, 2023 10:08
@NachoPal
Copy link
Contributor

bot fmt

@command-bot
Copy link

command-bot bot commented Jun 13, 2023

@NachoPal https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2977408 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/cumulus/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 4-41b932fa-987d-45bb-9e01-bbb34c3c7263 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jun 13, 2023

@NachoPal Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2977408 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2977408/artifacts/download.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez marked this pull request as draft June 13, 2023 12:15
@NachoPal
Copy link
Contributor

If the actual spent PoV depends on benchmarked weights. Does it mean we shouldn't regenerate the Weights for this calculations to remain correct?

@ggwpez
Copy link
Member Author

ggwpez commented Jun 13, 2023

If the actual spent PoV depends on benchmarked weights. Does it mean we shouldn't regenerate the Weights for paritytech/substrate#14338 (comment) calculations to remain correct?

Yea I will call the benchbot, thanks for noticing.
There should be no change, but lets see.

PS: The upstream CI is green now, so this MR should be good.

@ggwpez ggwpez marked this pull request as ready for review June 13, 2023 12:55
@NachoPal
Copy link
Contributor

Yea I will call the benchbot, thanks for noticing.

The runtime is using local weights though. If you want to update them you should run the benchbot also here

@ggwpez
Copy link
Member Author

ggwpez commented Jun 13, 2023

The runtime is using local weights though. If you want to update them you should run the benchbot also here

I hope weights are updated prior to releases anyway?
But can still do once upstream merged. Only downside is that this will block the Cumulus CI check for 2-3 hours for everyone.

@ggwpez
Copy link
Member Author

ggwpez commented Jun 13, 2023

Maybe I can also diener it and call the bot directly here, will try.

@NachoPal
Copy link
Contributor

I hope weights are updated prior to releases anyway?

Yes, they will be, but we are not waiting for next release to deploy Gluttons, we get the runtime from master.

I would just leave the weights as they are.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Command 'Command { std: cd "/storage/repositories/cumulus" && "git" "push" "paritytech" "oty-glutton-overunity", kill_on_drop: false }' failed with status Some(1); output: To https://github.com/paritytech/cumulus.git
! [rejected] oty-glutton-overunity -> oty-glutton-overunity (fetch first)
error: failed to push some refs to 'https://github.com/paritytech/cumulus.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

@ggwpez
Copy link
Member Author

ggwpez commented Jun 13, 2023

bot merge

The bot did not try to update. So doing it manually now.

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 015ac5d into master Jun 13, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-glutton-overunity branch June 13, 2023 16:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A4-companion A PR that should be considered alongside another (usually more comprehensive and detailed) PR. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants