Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
Suggest reviewing latest comments in #1927. |
@ltfschoen based on our documentation chat yesterday, we are not following that template in paritytech/substrate-developer-hub#44 (comment) because we don't expect someone who modifies the code to modify the readme. Therefore, specific function behavior (including interfaces) goes in the rustdocs and the README.adoc file is for higher-level information and usage examples. |
|
Latest commit for conflict resolution after #2048 |
joepetrowski
dismissed
shawntabrizi’s stale review
lots of changes since review
| //! | ||
| //! The `Call` enum is documented [here](https://crates.parity.io/srml_balances/enum.Call.html). | ||
| //! | ||
| //! - `transfer` - Transfer some liquid free balance to another account. |
There was a problem hiding this comment.
I appreciate the unified quite full explaining top documentation but at the same time I feel like it may result in outdated documentation or unnecessary duplication, here I wonder if it not better to change to the same kind of redirection as module below
There was a problem hiding this comment.
Yeah, that's why I kept the definitions really short here, so the detail is in the function documentation. I don't have a strong opinion about including them here or not, which is why I put the link to the Call documentation before the list.
Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com>
Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com>
Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com>
Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com>
| //! } | ||
| //! } | ||
| //! } | ||
| //! ``` |
There was a problem hiding this comment.
I kind of agree for this example but at the same time it is not specific to balance, so I'm mixed but ok
There was a problem hiding this comment.
Agree, not highly specific to balances. I thought our idea was to have an example with everything necessary to compile and use the module. Can remove if we think the real-use example is adequate.
gavofyork
left a comment
gavofyork
left a comment
There was a problem hiding this comment.
I'll approve since there's nothing wrong with committing as-is, but you might want to bring back the contracts example.
| //! pub struct Module<T: Trait> for enum Call where origin: T::Origin { | ||
| //! fn transfer_proxy(origin, to: T::AccountId, value: T::Balance) -> Result { | ||
| //! let sender = ensure_signed(origin)?; | ||
| //! <balances::Module<T>>::make_transfer(&sender, &to, value)?; |
There was a problem hiding this comment.
I think the updated function name is transfer() instead of make_transfer
There was a problem hiding this comment.
Yeah, and more generally throughout some of the early docs whose modules had a refactor. #2159
* comment updates * added rustdoc and readme * clarified LockableCurrency trait * Currency trait rustdocs * fixed typo * fixed suggestions round 1 * UpdateBalanceOutcome docs (open for discussion) * rm description of enum, consolidation, rm ReclaimRebate * type clarification, examples overhaul, adoc formatting * adoc to md * format change for rustdoc * update links and fix typos * typos and links * updates according to comments * new example * small clarifications * trait implementation section * missing ``` * small changes, ready for review * line width update * small tweaks * Update srml/balances/src/lib.rs Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com> * Update srml/balances/src/lib.rs Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com> * Update srml/balances/src/lib.rs Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com> * Update srml/balances/src/lib.rs Co-Authored-By: joepetrowski <25483142+joepetrowski@users.noreply.github.com> * Update lib.rs * address review by thiolliere * remove common warning * Update docs * updated srml example
12 participants
First attempt at documentation for the balances module. Besides a general review, this needs:
TODO: Add documentation for tests. There are a lot of them and I'd like better direction on the scope of rustdocs before completing them, so wanted to post for review at this stage.