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

Conversation

@gui1117
Copy link
Contributor

@gui1117 gui1117 commented Mar 21, 2019

  • decl event support rust doc for RawEvent, so I moved docs accordingly
  • improve decl_storage instance doc

/// circumstances that have happened that users, Dapps and/or chain explorers would find
/// interesting and otherwise difficult to detect.
decl_event!(
/// An event in this module.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's agree on one correct comment to go here, before the pub enum Event and use it for each module. I think "/// Events for this module." would be ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,
(as a note: if no module need to have a more explicit description, we can make it generated by the macro and remove the ability to customize)

Copy link
Contributor

Choose a reason for hiding this comment

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

That is probably better, because the notion of "events" could change, and we don't want to update the same comment in the entire SRML. There is documentation (at least in progress) on the Event type. Perhaps it makes sense to have a more thorough comment in srml/example/src/lib.rs and node-template/runtime/src/template.rs. Everything else should be consistent unless there is a unique feature to a module.

Copy link
Member

Choose a reason for hiding this comment

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

+1 for macro generation

Copy link
Contributor Author

@gui1117 gui1117 Mar 22, 2019

Choose a reason for hiding this comment

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

macro generation now as such:

/// Events for this module.
///
/// (other doc of user if some)

@gui1117 gui1117 added the A3-in_progress Pull request is in progress. No review needed at this stage. label Mar 21, 2019
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.

Some minor nitpicks.

@gui1117 gui1117 added A8-looksgood and removed A3-in_progress Pull request is in progress. No review needed at this stage. A4-gotissues labels Mar 25, 2019
ltfschoen added a commit that referenced this pull request Mar 25, 2019
@gavofyork gavofyork merged commit dc4b205 into master Mar 26, 2019
@gavofyork gavofyork deleted the gui-improve-docs branch March 26, 2019 13:36
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Apr 12, 2019
* improve decl_storage instance doc

* use decl_event doc accordignly

* automate doc and while allow to extend it on event

* some missed ones

* Update srml/example/src/lib.rs

Co-Authored-By: thiolliere <[email protected]>

* Update srml/support/procedural/src/storage/transformation.rs
gavofyork pushed a commit that referenced this pull request Apr 23, 2019
* WIP - SRML Assets Module README

* docs: Tweaks for consistency

* docs: Add missing newline

* review-fix: Remove non-SRML trait dependencies

* review-fix: Replace const with let

* review-fix: Remove use of compact in signature

* review-fix: Change const to let since cannot use result of function call

* fix: Add backticks around type and mention type it derives from

* review-fix: Update variable names since changed to lowercase since using let

* fix: Change type to bold instead of code

* review-fix: Update Asset module

* refactor: Consistent bullet points. Remove whitespace between items

* review-fix: Remove useless blah

* review-fix: Remove Storage Items

* review-fix: Remove Types

* review-fix: Remove duplicate instructions

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* review-fix: Remove since will be replaced after macro expansion #2068 as per comment

* review-fix: Move Goals within overview

* fix: Fix indentation

* style and a few minor changes

* remove Events

* capitalization

* docs: Reword the Goals to remove mention of cold wallets based on discussion with Joe

* Wording

* Update lib.rs

* Update lib.rs

* Update lib.rs
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Jul 10, 2019
* WIP - SRML Assets Module README

* docs: Tweaks for consistency

* docs: Add missing newline

* review-fix: Remove non-SRML trait dependencies

* review-fix: Replace const with let

* review-fix: Remove use of compact in signature

* review-fix: Change const to let since cannot use result of function call

* fix: Add backticks around type and mention type it derives from

* review-fix: Update variable names since changed to lowercase since using let

* fix: Change type to bold instead of code

* review-fix: Update Asset module

* refactor: Consistent bullet points. Remove whitespace between items

* review-fix: Remove useless blah

* review-fix: Remove Storage Items

* review-fix: Remove Types

* review-fix: Remove duplicate instructions

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* Update srml/assets/src/lib.rs

Co-Authored-By: ltfschoen <[email protected]>

* review-fix: Remove since will be replaced after macro expansion paritytech#2068 as per comment

* review-fix: Move Goals within overview

* fix: Fix indentation

* style and a few minor changes

* remove Events

* capitalization

* docs: Reword the Goals to remove mention of cold wallets based on discussion with Joe

* Wording

* Update lib.rs

* Update lib.rs

* Update lib.rs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants