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

fix: typos #1568

Merged
merged 6 commits into from
Nov 7, 2023
Merged

fix: typos #1568

merged 6 commits into from
Nov 7, 2023

Conversation

omahs
Copy link
Contributor

@omahs omahs commented Sep 25, 2023

fix: typos

@omahs omahs requested a review from lemunozm as a code owner September 25, 2023 10:37
@mustermeiszer mustermeiszer added the D0-ready Pull request can be merged without special precaution and notification. label Sep 25, 2023
lemunozm
lemunozm previously approved these changes Sep 25, 2023
Copy link
Contributor

@lemunozm lemunozm left a comment

Choose a reason for hiding this comment

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

Thanks for the corrections! 🙌🏻

@NunoAlexandre NunoAlexandre enabled auto-merge (squash) September 26, 2023 09:33
Copy link
Contributor

@NunoAlexandre NunoAlexandre left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! One thing to fix

@@ -43,15 +43,15 @@ This must be done before releasing a new version of any of our runtimes to ensur

- Click on **propose**

1. Voting phase. You should make 3 votes in favor with 3 diferent people (modify the CFG amount before to a low number).
1. Voting phase. You should make 3 votes in favor of 3 different people (modify the CFG amount before to a low number).
Copy link
Contributor

Choose a reason for hiding this comment

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

That correction is not actually correct, since the desired semantic here is that 3 different accounts vote Aey, note that someone "should make 3 votes in favour of 3 different people".

Suggested change
1. Voting phase. You should make 3 votes in favor of 3 different people (modify the CFG amount before to a low number).
1. Voting phase. Three different account should vote in favour of the proposal (modify the CFG amount before to a low number).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also not sure we need 3 separate 👍 , 1 should be enough? 🤔 cc @wischli

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also not sure we need 3 separate 👍 , 1 should be enough? 🤔 cc @wischli

Our local chain specs setup 4 genesis council members

fn council_members_bootstrap() -> Vec<cfg_primitives::AccountId> {
endowed_accounts().into_iter().take(4).collect()
}

Since both proposing an external referendum as well as fast-tracking requires at least half of the council, two votes would suffice. I suppose this is a remnant of previously requiring at least 75% of the council.

On that note, our runtime upgrade scripts votes with three members:

console.log("Continuing with council vote using", result[0], result[1])
nonce = await getNonce(api, alice.address);
await councilVoteProposal(api, alice, result[0], result[1], nonce, false)
nonce = await getNonce(api, bob.address);
await councilVoteProposal(api, bob, result[0], result[1], nonce, false)
nonce = await getNonce(api, charlie.address);
await councilVoteProposal(api, charlie, result[0], result[1], nonce, true)

Copy link
Contributor

Choose a reason for hiding this comment

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

The number of votes must match the threshold set when proposing. The document currently states 3 (75%) which is outdated

1. In Governance -> Council -> Motions, click on **propose motion**.
-    - At **umbral**, put `3` (a 75% of the total which is 4).
+    - At **umbral**, put `2` (a 50% of the total which is 4).

wischli
wischli previously approved these changes Sep 28, 2023
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

Thanks for fixing our typos! As stated, some content of the readme should be updated as well most ideally. However, I am fine with merging these fixes as the content is not incorrect and fixing the details is out of scope of this PR in my opinion.

docs/runtime-upgrade.md Outdated Show resolved Hide resolved
@omahs omahs dismissed stale reviews from wischli and lemunozm via 71767b1 September 29, 2023 09:54
Co-authored-by: Nuno Alexandre <[email protected]>
Copy link
Contributor

@NunoAlexandre NunoAlexandre left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution 🙏

@mustermeiszer
Copy link
Collaborator

@omahs still interested in bringing this in?

@omahs
Copy link
Contributor Author

omahs commented Nov 6, 2023

@mustermeiszer Hey, yes, should I edit something?

Copy link
Collaborator

@mustermeiszer mustermeiszer left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your contribution.

Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@mustermeiszer mustermeiszer merged commit 1bfc4bf into centrifuge:main Nov 7, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D0-ready Pull request can be merged without special precaution and notification.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants