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.

Sign up for GitHub

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

Jump to bottom

staking: small README improvements #502

Merged
merged 2 commits into from
Aug 27, 2024
Merged

staking: small README improvements #502

merged 2 commits into from
Aug 27, 2024

Conversation

feuGeneA
Copy link
Contributor

@feuGeneA feuGeneA commented Aug 23, 2024 •
edited
Loading

Why this should be merged

  • ensures all hyperlinks are applied to the first appearance of a term in the doc
  • adds some hyperlinks that were missing (things we can link to but weren't yet)
  • corrects SetValidatorWeightTx to SetSubnetValidatorWeightTx
  • changes some more instances of "staking manager" to "validator manager"
  • adds to message format descriptions a mention of where the message is issued from
  • makes a few tiny grammar/style improvements

@feuGeneA
staking: small README improvements
890ac36 
Signed-off-by: F. Eugene Aumson <[email protected]>
@feuGeneA feuGeneA marked this pull request as ready for review August 23, 2024 18:14
@feuGeneA feuGeneA requested a review from a team as a code owner August 23, 2024 18:14
cam-schultz
cam-schultz approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@cam-schultz cam-schultz left a comment

Choose a reason for hiding this comment

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

One nit, otherwise LGTM

contracts/staking/README.md Outdated

Signed by: Subnet
Signed by: Subnet validators
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It's a bit odd that we specify the subnet validators here, but not the P-Chain validators in other sections. If we're going to be specific, the other sections should also be Signed by the subnet validators, via their P-Chain VM due to the self-signing optimization for Warp messages from the primary network.

IMO it's clearer to leave this as is, and omit validators altogether.

@feuGeneA
fix nit
7414d70 
addresses review comment #502 (comment)

Signed-off-by: F. Eugene Aumson <[email protected]>
@feuGeneA feuGeneA merged commit 5cd47af into staking-contract Aug 27, 2024
12 checks passed
@michaelkaplan13 michaelkaplan13 deleted the staking-contract-readme-tweaks branch September 20, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iansuvak iansuvak iansuvak approved these changes

@geoff-vball geoff-vball geoff-vball approved these changes

@bernard-avalabs bernard-avalabs bernard-avalabs approved these changes

@cam-schultz cam-schultz cam-schultz approved these changes

@minghinmatthewlam minghinmatthewlam Awaiting requested review from minghinmatthewlam minghinmatthewlam was automatically assigned from ava-labs/interop

@michaelkaplan13 michaelkaplan13 Awaiting requested review from michaelkaplan13 michaelkaplan13 was automatically assigned from ava-labs/interop

Assignees
No one assigned
Labels
None yet
Projects
Platform Engineering Group
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@feuGeneA @iansuvak @geoff-vball @bernard-avalabs @cam-schultz