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

Documentation for the Democracy Module#2286

Merged
bkchr merged 18 commits intov1.0from
luke-democracy-module-docs
Jul 9, 2019
Merged

Documentation for the Democracy Module#2286
bkchr merged 18 commits intov1.0from
luke-democracy-module-docs

Conversation

@ltfschoen
Copy link
Contributor

@ltfschoen ltfschoen commented Apr 15, 2019

Relates to polkadot-developers/substrate-developer-hub.github.io#28

Requires update for consistency with #2172

@ltfschoen ltfschoen added the A3-in_progress Pull request is in progress. No review needed at this stage. label Apr 15, 2019
@joepetrowski
Copy link
Contributor

A lot of code exceeded our style guide length limit. Fixed in most recent commit, but can be rolled back if we want this to be strictly comments.

@joepetrowski joepetrowski added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Apr 17, 2019
@ltfschoen ltfschoen force-pushed the luke-democracy-module-docs branch from 064d511 to e955fcb Compare April 18, 2019 13:50
@joepetrowski joepetrowski requested a review from gavofyork April 18, 2019 13:50
@ltfschoen
Copy link
Contributor Author

@joepetrowski the indentation didn't look right in the code snippet when i ran cargo doc --package srml-democracy --open. it was ok on lines where indentation had been done using spaces, but not when indentation had been done using tabs, so i just removed the tabs and replaced them with spaces

Copy link

@DemiMarie-temp DemiMarie-temp left a comment

Choose a reason for hiding this comment

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

The specific threshold for a supermajority should be specified.

/// A supermajority of approvals is needed to pass this vote.
SuperMajorityApprove,
/// A supermajority of rejects is needed to fail this vote.
/// A supermajority of rejections is needed to fail this vote.

Choose a reason for hiding this comment

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

How large a supermajority is needed? This should be documented.

Copy link
Contributor

Choose a reason for hiding this comment

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

See update

@joepetrowski joepetrowski removed the A0-please_review Pull request needs code review. label Jun 11, 2019
@ltfschoen
Copy link
Contributor Author

i've received feedback from rpmeier suggesting that we providing information about what origin the proposed call is executed with

@joepetrowski joepetrowski changed the base branch from master to v1.0 June 30, 2019 18:45
@shawntabrizi shawntabrizi force-pushed the luke-democracy-module-docs branch from df1c83c to 667f29d Compare July 3, 2019 09:38
@ltfschoen
Copy link
Contributor Author

ltfschoen commented Jul 3, 2019

@shawntabrizi why are you force pushing and deleting all the commit history from other contributors? why not just ask me to close this PR and create a fresh branch of your own instead of discrediting our work?

@shawntabrizi
Copy link
Member

@ltfschoen I am not intending to discredit any work here, but these docs have been stale and were originally opened to merge to the master branch.

Since the SRML Democracy module has changed quite a bit in master, we re-targeted this branch to merge to v1.0.

However, at certain points this branch was re-based or merged with the master branch which means that when we changed the base branch to v1.0, we were getting changes on the order of 400+ files changed.

I isolated the changes and fixed up the PR. There could probably be a better way to do this, but end of the day, I just want to get these docs merged in.

@shawntabrizi shawntabrizi added A0-please_review Pull request needs code review. and removed A4-gotissues labels Jul 4, 2019
@ltfschoen
Copy link
Contributor Author

Fair enough, thanks for cleaning it up!

Copy link
Contributor Author

@ltfschoen ltfschoen left a comment

Choose a reason for hiding this comment

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

I've left review comments with various nitpicks.

@ltfschoen
Copy link
Contributor Author

With regard to the actual implementation, I came up with the following questions:

  1. In the implementation of set_proxy https://github.com/paritytech/substrate/pull/2286/files#diff-cbe2b253de4fb4f87ccc41b9075568d6R305, how are we checking that it is actually being called by a stash account as expected (rather than being called by a proxy account for instance)? do we need an additional assertion?

  2. In the implementation of resign_proxy https://github.com/paritytech/substrate/pull/2286/files#diff-cbe2b253de4fb4f87ccc41b9075568d6R312, how are we checking that it is actually being called by a proxy account as expected (rather than being called by a stash account for instance)? do we need an additional assertion?

  3. In the implementation of remove_proxy https://github.com/paritytech/substrate/pull/2286/files#diff-cbe2b253de4fb4f87ccc41b9075568d6L176, how are we checking that it is actually being called by a stash account as expected (rather than being called by a proxy account for instance)? do we need an additional assertion?

shawntabrizi and others added 3 commits July 5, 2019 01:59
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
shawntabrizi and others added 9 commits July 5, 2019 02:00
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
Co-Authored-By: Luke Schoen <ltfschoen@users.noreply.github.com>
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.

A few small fixes, otherwise LGTM.

shawntabrizi and others added 2 commits July 7, 2019 14:00
Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
@bkchr bkchr merged commit 2e30b73 into v1.0 Jul 9, 2019
@bkchr bkchr deleted the luke-democracy-module-docs branch July 9, 2019 11:19
bkchr added a commit that referenced this pull request Jul 9, 2019
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants