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

WeightMeter: more consistent naming #14586

Merged
merged 11 commits into from
Jul 18, 2023
Merged

WeightMeter: more consistent naming #14586

merged 11 commits into from
Jul 18, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Jul 17, 2023 •
edited
Loading

@gavofyork recommended some more consistent naming for the WeightMeter. Breaking changes are prefixed with 🚨

Changes:

  • Deprecate defensive_saturating_accrue and introduce consume instead.
  • Deprecate check_accrue and introduce try_consume instead.
  • Deprecate can_accrue and introduce can_consume instead.
  • 🚨 Privatize consumed and limit and add consumed() and limit() getters instead.

Polkadot Companion: paritytech/polkadot#7507

ggwpez added 3 commits July 17, 2023 11:03
@ggwpez
Rename WeightMeter functions
ff00f4b
@ggwpez
Fixes
3ff3c46 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Fixup and doc + tests
7d0b904 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. labels Jul 17, 2023
@ggwpez ggwpez self-assigned this Jul 17, 2023
ggwpez added 2 commits July 17, 2023 11:53
@ggwpez
One more test
373bc6f 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Fixup pallets
66bff34 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez requested review from a team July 17, 2023 10:40
@ggwpez
Use correct function 🤦
7d70bda 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
This was referenced Aug 24, 2023
Open
Closed
@juangirini juangirini changed the title WeightMeter: more consistent naming [Deprecation] WeightMeter: more consistent naming Jul 17, 2023
@juangirini juangirini changed the title [Deprecation] WeightMeter: more consistent naming WeightMeter: more consistent naming Jul 17, 2023
juangirini
juangirini approved these changes Jul 17, 2023
View reviewed changes
Copy link
Contributor

@juangirini juangirini 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 nits

primitives/weights/src/weight_meter.rs Outdated Show resolved Hide resolved
primitives/weights/src/weight_meter.rs Outdated Show resolved Hide resolved
primitives/weights/src/weight_meter.rs Outdated

/// Consume some weight and defensively fail if it is over the limit. Saturate in any case.
///
/// This is provided as a less noisy version of `defensive_saturating_accrue`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The meaning of a "less noisy version" is not clear to me. I mean, defensive_saturating_accrue is just an alias now. Maybe we can just remove this line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea I copied if from the Gist from Gav - but agreed; it is unnecessary to mention now.

bkchr
bkchr approved these changes Jul 17, 2023
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

The new naming is soo much better. ❤️

primitives/weights/src/weight_meter.rs Outdated Show resolved Hide resolved
ggwpez and others added 3 commits July 17, 2023 17:13
@ggwpez @juangirini
Apply suggestions from code review
9961b60 
Co-authored-by: Juan <juangirini@gmail.com>
@ggwpez @bkchr
Update primitives/weights/src/weight_meter.rs
3210272 
Co-authored-by: Bastian Köcher <git@kchr.de>
@ggwpez @bkchr
Update primitives/weights/src/weight_meter.rs
339d294 
Co-authored-by: Bastian Köcher <git@kchr.de>
@ggwpez ggwpez added D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit and removed D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Jul 18, 2023
@ggwpez
Copy link
Member Author

ggwpez commented Jul 18, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit cfe2432 into master Jul 18, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-rename-weightmeter branch July 18, 2023 14:38
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@ggwpez @juangirini
@bkchr @nathanwhit
WeightMeter: more consistent naming (paritytech#14586)
65d7bdc 
* Rename WeightMeter functions

* Fixes

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

* Fixup and doc + tests

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

* One more test

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

* Fixup pallets

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

* Use correct function 🤦

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

* Apply suggestions from code review

Co-authored-by: Juan <juangirini@gmail.com>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update primitives/weights/src/weight_meter.rs

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Juan <juangirini@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Ank4n pushed a commit that referenced this pull request Jul 22, 2023
@ggwpez @juangirini
@bkchr @Ank4n
WeightMeter: more consistent naming (#14586)
4210b52 
* Rename WeightMeter functions

* Fixes

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

* Fixup and doc + tests

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

* One more test

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

* Fixup pallets

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

* Use correct function 🤦

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

* Apply suggestions from code review

Co-authored-by: Juan <juangirini@gmail.com>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update primitives/weights/src/weight_meter.rs

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Juan <juangirini@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
@JesseAbram JesseAbram mentioned this pull request Oct 26, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@juangirini juangirini juangirini approved these changes

Assignees

@ggwpez ggwpez

Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the 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 T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Runtime / FRAME
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ggwpez @juangirini @bkchr