Improve documentation for `fast-unstake` pallet #14101

kianenigma · 2023-05-09T10:39:18Z

This is my effort to try and improve the documentation of a single pallet, in order to identify some patters that we can try and enforce across all pallets. Most of my efforts was around making the experience of navigating the rust-docs better.

Should close #13299 with collaboration with @ggwpez
relates to paritytech/polkadot-sdk-docs#1
Should go hand-in-hand with #13987 which is improving some macros to improve storage docs.

Look into Oliver's ideas in MQ pallet (among few other well-documented pallets)
Done in add new documentation guidelines #14115:
- Bring over https://hackmd.io/mRrrewoxTOyfKJ2H16R2xw into our contributing guides.
- Bring over https://hackmd.io/4pjRqPNNTnGXJHKK9jv90A into our contributing guides.
- make https://github.com/orgs/paritytech/teams/docs-audit be a reviewer of the select crates.

Some of the things that I have done in macros:

Add an auto-generated doc inside mod pallet that tells you where you should go. This can also live in the top level file, perhaps, but then we have to repeat it all the time.
Call docs now point to the right Pallet function.
everything in mod dispatchables also points to Pallet. Both of these is to establish the fact that the "real thing" is the method defined on Pallet.
For storage types, similar rule holds in mod storage_types, but if the storage is private, we copy-paste the doc over.
Similarly, for getters, if the storage is public, we link to it, else, we copy-paste the doc over.
Showcase how to write at least a few tests that go into the top level doc of a pallet.
Inject a doc-line to each storage type, showcasing its type and keys and so on.
showcase a new frame_support::testing_prelude that can be used.

sam0x17

This looks great, let me push out another version of docify that uses the original span locations so we get exact source text and then should be good RE your issue you showed me of comments not copying over. Will also remove the need for prettyplease dependency

frame/fast-unstake/src/lib.rs

frame/support/procedural/src/pallet/expand/mod.rs

frame/support/procedural/src/pallet/expand/storage.rs

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

…/substrate into kiz-improve-fast-unstake-dcos

frame/fast-unstake/src/lib.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

sam0x17 · 2023-05-23T12:24:14Z

btw, also planning on doing a thing eventually that by default will make it so you don't have to write #[docify::export] unless you need to specify a disambiguation name. This will also coincide with me properly resolving module paths within a file, so you'll be able to do docify::embed!("my/path.rs", some::path::to::MyItem) and it will resolve correctly. It just means I will have to modify my visitor pattern to be aware of what submodules it is in within a file

…/substrate into kiz-improve-fast-unstake-dcos

muharem · 2023-05-26T14:08:44Z

frame/support/procedural/src/pallet/expand/mod.rs

+The main components of this pallet are:
+- [`Pallet`], which implements all of the dispatchable extrinsics of the pallet, among
+other public functions.
+	- The subset of the functions that are dispatchable can be identified either in


Suggested change

- The subset of the functions that are dispatchable can be identified either in

- The subset of the functions that are dispatchable can be identified either in

It should be an indented list, why are you suggesting otherwise?

frame/support/procedural/src/pallet/expand/doc_only.rs

frame/support/procedural/src/pallet/expand/config.rs

mrcnski · 2023-05-26T15:09:21Z

frame/fast-unstake/src/lib.rs

+//!
+//! ## Overview
+//!
+//! This pallet that's designed to JUST do the following:


Suggested change

//! This pallet that's designed to JUST do the following:

//! This pallet is designed to JUST do the following:

Although TBH I'm not sure what purpose this line serves, can it be removed?

Also, since we are improving the docs: I feel like the text that follows could use some structure, e.g. subsections or bullet points. Right now it seems (to me) like an unapproachable wall of text.

I find your comment about this sentence fair, but not the rest. It is organized into a series of paragraphs and is following a logical chain. Perhaps if you describe further which part of it is unapprochable, I can improve.

Nonetheless though, this is in realm of subjective measures harder to align on. I don't think we can ever make everyone write docs that are understandable to everyone. Simply things like different ways of thinking and language levels will make that impossible.

What we want to ensure is at least following the structure mentioned in #14115, and doing your best.

Fair enough, it was indeed a subjective impression I got as someone new to this pallet. Reading my post again it sounds more critical than I intended it to be, sorry. 🙈 To be concrete, I would suggest adding subsections like e.g. "Registration" and "Checking". Feel free to ignore if you think that's unnecessary. The new docs are already awesome as is!

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

…/substrate into kiz-improve-fast-unstake-dcos

sam0x17 · 2023-05-28T17:37:47Z

docify 0.1.11 should fix the termcolor issue

kianenigma · 2023-05-29T10:28:06Z

bot rebase

…stake-dcos

paritytech-processbot · 2023-05-29T10:28:12Z

Rebased

kianenigma · 2023-05-29T10:28:42Z

bot merge

paritytech-processbot · 2023-05-29T10:28:49Z

Waiting for commit status.

paritytech-processbot · 2023-05-29T10:39:26Z

Merge cancelled due to error. Error: Statuses failed for 351ee05

sam0x17 · 2023-05-29T14:50:29Z

frame/support/src/lib.rs

@@ -1520,6 +1520,17 @@ pub mod tests {
 	}
 }

+/// Prelude to be used for pallet testing, for ease of use.
+#[cfg(feature = "std")]
+pub mod testing_prelude {


Do we want to use the same naming in #13454?

…rove-fast-unstake-dcos

kianenigma · 2023-05-29T17:44:34Z

bot merge

* improve documentation of fast-unstake pallet * using Sam's crate now * fix * remove stuff not needed * Some updates * use new prelude. * update * some other fancy docs * Update frame/fast-unstake/src/lib.rs Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> * Update frame/support/procedural/src/pallet/expand/mod.rs Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> * update * Update frame/fast-unstake/src/lib.rs Co-authored-by: Keith Yeung <kungfukeith11@gmail.com> * update * fix no_std issue by updating to latest version of docify * get things compiling by adding a use for StakingInterface * fix docify paths to their proper values, still not working because bug * embed working 🎉 * update syn * upgrade to docify v0.1.10 for some fixes * Apply suggestions from code review Co-authored-by: Juan <juangirini@gmail.com> Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> * improve docs * Update frame/support/procedural/src/pallet/expand/doc_only.rs Co-authored-by: Juan <juangirini@gmail.com> * fmt * fix * Update frame/support/procedural/src/pallet/expand/doc_only.rs Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com> * Update frame/support/procedural/src/pallet/expand/config.rs Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com> * Update frame/support/procedural/src/pallet/expand/config.rs Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com> * remove redundant * update docify rev * update. * update. * update lock file --------- Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> Co-authored-by: Keith Yeung <kungfukeith11@gmail.com> Co-authored-by: Sam Johnson <sam@durosoft.com> Co-authored-by: Juan <juangirini@gmail.com> Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com> Co-authored-by: parity-processbot <>

ggwpez · 2023-08-13T16:22:41Z

frame/support/procedural/src/pallet/expand/storage.rs

@@ -310,6 +311,65 @@ pub fn process_generics(def: &mut Def) -> syn::Result<Vec<ResultOnEmptyStructMet
 	Ok(on_empty_struct_metadata)
 }

+fn augment_final_docs(def: &mut Def) {


@kianenigma this actually prevents #[deny(missing_docs)] from working correctly for storage items, since they now always have a default doc comment.
We could only do this if there is already a comment, or always emit a warning when there is not doc. WDYT?

kianenigma and others added 6 commits May 7, 2023 15:31

improve documentation of fast-unstake pallet

dbf2572

using Sam's crate now

9d2a9a2

merge

bb11dc0

fix

91e811b

remove stuff not needed

85f4390

Some updates

1a6c63d

kianenigma marked this pull request as ready for review May 9, 2023 10:42

kianenigma requested review from a team, sam0x17 and ggwpez May 9, 2023 10:42

sam0x17 reviewed May 9, 2023

View reviewed changes

use new prelude.

dc82e48

liamaharon self-requested a review May 9, 2023 15:31

ggwpez reviewed May 9, 2023

View reviewed changes

frame/fast-unstake/src/lib.rs Show resolved Hide resolved

frame/fast-unstake/src/lib.rs Outdated Show resolved Hide resolved

frame/fast-unstake/src/lib.rs Outdated Show resolved Hide resolved

frame/fast-unstake/src/lib.rs Show resolved Hide resolved

liamaharon removed their request for review May 10, 2023 06:38

update

4973a32

kianenigma requested a review from a team May 10, 2023 08:21

liamaharon reviewed May 10, 2023

View reviewed changes

kianenigma and others added 5 commits May 10, 2023 10:59

some other fancy docs

8d0c523

Update frame/fast-unstake/src/lib.rs

1572dd8

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

Update frame/support/procedural/src/pallet/expand/mod.rs

9aa4ede

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

update

f24ac3c

Merge branch 'kiz-improve-fast-unstake-dcos' of github.com:paritytech…

048e061

…/substrate into kiz-improve-fast-unstake-dcos

kianenigma mentioned this pull request May 10, 2023

add new documentation guidelines #14115

Merged

KiChjang reviewed May 10, 2023

View reviewed changes

frame/fast-unstake/src/lib.rs Outdated Show resolved Hide resolved

kianenigma and others added 2 commits May 10, 2023 19:24

Update frame/fast-unstake/src/lib.rs

91846b9

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

Master.into()

99cc532

paritytech-ci requested a review from a team May 23, 2023 08:29

update

6b7804a

kianenigma added 3 commits May 26, 2023 15:11

fmt

9c9ab28

Merge branch 'kiz-improve-fast-unstake-dcos' of github.com:paritytech…

8976e53

…/substrate into kiz-improve-fast-unstake-dcos

fix

396cec2

muharem approved these changes May 26, 2023

View reviewed changes

mrcnski reviewed May 26, 2023

View reviewed changes

kianenigma and others added 5 commits May 28, 2023 10:40

Update frame/support/procedural/src/pallet/expand/doc_only.rs

21fc4df

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

Update frame/support/procedural/src/pallet/expand/config.rs

e4939dc

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

Update frame/support/procedural/src/pallet/expand/config.rs

ffe579f

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

remove redundant

04a5aff

Merge branch 'kiz-improve-fast-unstake-dcos' of github.com:paritytech…

9d7fb36

…/substrate into kiz-improve-fast-unstake-dcos

update docify rev

ff78017

Merge remote-tracking branch 'origin/master' into kiz-improve-fast-un…

351ee05

…stake-dcos

kianenigma added 2 commits May 29, 2023 12:44

update.

9602902

update.

93aca7a

sam0x17 approved these changes May 29, 2023

View reviewed changes

kianenigma added 2 commits May 29, 2023 17:00

update lock file

55fc001

Merge branch 'master' of github.com:paritytech/substrate into kiz-imp…

851e4af

…rove-fast-unstake-dcos

paritytech-processbot bot merged commit e5670d2 into master May 29, 2023

paritytech-processbot bot deleted the kiz-improve-fast-unstake-dcos branch May 29, 2023 17:44

ggwpez reviewed Aug 13, 2023

View reviewed changes

ggwpez mentioned this pull request Aug 24, 2023

Auto-generated docs prevent missing_docs lint paritytech/polkadot-sdk#132

Open

kianenigma mentioned this pull request Feb 28, 2024

Fix call enum's metadata regression paritytech/polkadot-sdk#3513

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve documentation for `fast-unstake` pallet #14101

Improve documentation for `fast-unstake` pallet #14101

kianenigma commented May 9, 2023 •

edited

Loading

sam0x17 left a comment

sam0x17 commented May 23, 2023

muharem May 26, 2023

kianenigma May 28, 2023

mrcnski May 26, 2023

kianenigma May 28, 2023

mrcnski May 28, 2023

sam0x17 commented May 28, 2023

kianenigma commented May 29, 2023

paritytech-processbot bot commented May 29, 2023

kianenigma commented May 29, 2023

paritytech-processbot bot commented May 29, 2023

paritytech-processbot bot commented May 29, 2023

sam0x17 May 29, 2023

kianenigma commented May 29, 2023

ggwpez Aug 13, 2023 •

edited

Loading

	- The subset of the functions that are dispatchable can be identified either in
	- The subset of the functions that are dispatchable can be identified either in

	//! This pallet that's designed to JUST do the following:
	//! This pallet is designed to JUST do the following:

Improve documentation for fast-unstake pallet #14101

Improve documentation for fast-unstake pallet #14101

Conversation

kianenigma commented May 9, 2023 • edited Loading

sam0x17 left a comment

Choose a reason for hiding this comment

sam0x17 commented May 23, 2023

muharem May 26, 2023

Choose a reason for hiding this comment

kianenigma May 28, 2023

Choose a reason for hiding this comment

mrcnski May 26, 2023

Choose a reason for hiding this comment

kianenigma May 28, 2023

Choose a reason for hiding this comment

mrcnski May 28, 2023

Choose a reason for hiding this comment

sam0x17 commented May 28, 2023

kianenigma commented May 29, 2023

paritytech-processbot bot commented May 29, 2023

kianenigma commented May 29, 2023

paritytech-processbot bot commented May 29, 2023

paritytech-processbot bot commented May 29, 2023

sam0x17 May 29, 2023

Choose a reason for hiding this comment

kianenigma commented May 29, 2023

ggwpez Aug 13, 2023 • edited Loading

Choose a reason for hiding this comment

Improve documentation for `fast-unstake` pallet #14101

Improve documentation for `fast-unstake` pallet #14101

kianenigma commented May 9, 2023 •

edited

Loading

ggwpez Aug 13, 2023 •

edited

Loading