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

Conversation

@ltfschoen
Copy link
Contributor

@ltfschoen ltfschoen added A0-please_review Pull request needs code review. M3-docs labels Mar 7, 2019
@ltfschoen
Copy link
Contributor Author

@bkchr I've pushed commits to address your review comments.

@kianenigma
Copy link
Contributor

@ltfschoen based on the new discussion progress the file should be changed to .md

@shawntabrizi
Copy link
Member

@ltfschoen Also the template has changed a bit, and I think this does not quite reflect that

@ltfschoen
Copy link
Contributor Author

ltfschoen commented Mar 20, 2019

@kianenigma @shawntabrizi @xlc @bkchr I've pushed a commit to address all comments.

Run cargo doc --package srml-assets --open to view the docs

@ltfschoen
Copy link
Contributor Author

ltfschoen commented Mar 25, 2019

@joepetrowski I've addressed your comments and the indentation. This comment is outstanding: #1945 (comment)

@joepetrowski
Copy link
Contributor

joepetrowski commented Mar 25, 2019

Changes LGTM, should get resolution on the "goals" section and approval from code author before merging.

@joepetrowski
Copy link
Contributor

Updated links, style, shortened a few things to match other modules. Should consider removing the ### Events section entirely as no other SRML module includes it.

@ltfschoen
Copy link
Contributor Author

@bkchr who would be most appropriate to get resolution on the "goals" section, and who would be the most appropriate code author to provide approval, as mentioned here #1945 (comment)

@bkchr
Copy link
Member

bkchr commented Apr 9, 2019

@ltfschoen this documentation is not following the standard like the others. Then there are code example that will not be tested by rustdoc.

@gavofyork
Copy link
Member

This is kind of pointless test code as it doesn't demonstrate how to use the three transaction functions. But I guess there's a bit more detail than before. I removed the obviously pointless bits of code (getters that didn't actually get) and will merge.

@gavofyork gavofyork merged commit fabf6e6 into master Apr 23, 2019
@gavofyork gavofyork deleted the luke-22-assets-module-docs branch April 23, 2019 17:59
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

A0-please_review Pull request needs code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants