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

[Documentation] Add description for VoteTally's methods #3140

Merged

Conversation

pandres95
Copy link
Contributor

@pandres95 pandres95 commented Jan 30, 2024

Description

While methods' names on VoteTally trait might be self-explanatory at first sight, the distinction between support and approval can be a bit ambiguous for some readers. This PR aims to clarify the distinction and inform about the expected values for every not yet documented method on this trait.

Checklist

  • My PR includes a detailed description as outlined in the "Description" section above
  • My PR follows the labeling requirements of this project (at minimum one label for T
    required)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@pandres95 pandres95 requested review from a team January 30, 2024 16:27
@pandres95 pandres95 force-pushed the pandres95--document-trait-votetally branch from 891f717 to bb29c3a Compare January 30, 2024 16:31
Copy link
Contributor

@joepetrowski joepetrowski left a comment

Choose a reason for hiding this comment

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

Sentences should have a period at the end.

While methods' names on VoteTally trait might be self-explanatory at first sight, the distinction between support and approval can be a bit ambiguous for some readers.

Isn't this explained in the Tally struct?

substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 30, 2024 17:17
@pandres95 pandres95 force-pushed the pandres95--document-trait-votetally branch 4 times, most recently from cc77ac6 to 4babf3c Compare January 30, 2024 17:20
@pandres95
Copy link
Contributor Author

Isn't this explained in the Tally struct?

It is explained, but specifically for the context of the conviction voting's Tally struct. Consider that, for example, RankedCollective's tally has an, although similar, different context, since conviction is not applied here. That's why I define it here in the trait as a multiplier, and give out some examples of what that might mean.

substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
substrate/frame/support/src/traits/voting.rs Outdated Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 30, 2024 22:29
@pandres95 pandres95 force-pushed the pandres95--document-trait-votetally branch from 6c6cdb1 to 60b2b89 Compare January 30, 2024 22:29
@bkchr bkchr added the R0-silent Changes should not be mentioned in any release notes label Jan 31, 2024
@pandres95 pandres95 requested a review from a team as a code owner January 31, 2024 19:19
@bkchr bkchr enabled auto-merge January 31, 2024 19:19
@bkchr bkchr added this pull request to the merge queue Jan 31, 2024
Merged via the queue into paritytech:master with commit 8a8f6f9 Jan 31, 2024
124 checks passed
@pandres95 pandres95 deleted the pandres95--document-trait-votetally branch March 10, 2024 03:06
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
)

# Description

While methods' names on [`VoteTally`][1] trait might be self-explanatory
at first sight, the distinction between `support` and `approval` can be
a bit ambiguous for some readers. This PR aims to clarify the
distinction and inform about the expected values for every not yet
documented method on this trait.

# Checklist

- [x] My PR includes a detailed description as outlined in the
"Description" section above
- [ ] My PR follows the [labeling requirements](CONTRIBUTING.md#Process)
of this project (at minimum one label for `T`
  required)
- [x] I have made corresponding changes to the documentation (if
applicable)
- [x] I have added tests that prove my fix is effective or that my
feature works (if applicable)

[1]:
https://docs.rs/frame-support/latest/frame_support/traits/trait.VoteTally.html

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants