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

macro stubs for all pallet:: macros to improve documentation visibility and discovery + revamp of pallet macro documentation #12334

Merged
merged 106 commits into from
Oct 6, 2022

Conversation

sam0x17
Copy link
Contributor

@sam0x17 sam0x17 commented Sep 23, 2022

Description

This PR adds macro stubs in frame_support_procedural for all of the pallet:: macros, moves documentation for the specific pallet:: macros to their respective macro stubs (note that we do keep the majority of it where it was, only things like expansion info are truly moved), and finally includes the stubs as part of the pallet prelude. Additionally this PR substantially upgrades and re-writes a lot of the pallet macro documentation. This results in several key developer experience improvements:

  1. In the generated cargo docs, searching for a particular pallet macro in the search bar or browsing through the list of macros will result in actually finding the macro you are looking for instead of it saying no results found
  2. Users of rust-analyzer and similar static analysis tools will see docs for pallet:: macros when they hover over them (right now they see nothing)
  3. Users of rust-analyzer and similar static analysis tools will be able to right click a pallet:: macro and select "Go to Definition" successfully (right now it fails to find the definition)

For context, the reason we don't already have stubs for these macros is we use them essentially as markers that later have their AST nodes removed by the pallet expansion macro. In other words definitions for these macros don't actually need to exist in our codebase for them to function properly, since all the code associated with these markers is elsewhere in the pallet expansion macro machinery.

Note I ended up also substantially re-writing/fixing up the existing pallet macro documentation, as noted in my line comments.

Related Issues

Proof of Concept

Screen Shot 2022-09-23 at 1 25 18 AM

@sam0x17 sam0x17 self-assigned this Sep 23, 2022
@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Sep 23, 2022
@sam0x17 sam0x17 added Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder C1-low PR touches the given topic and has a low impact on builders. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Sep 23, 2022
@sam0x17
Copy link
Contributor Author

sam0x17 commented Sep 27, 2022

Note that I've been going a step further with this -- it turns out the docs on that page had a lot of grammatical errors and formatting confusions so I've been fixing up the whole thing and adding a more navigable and complete table of all the pallet macros :)

Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

awesome, tyvm

@sam0x17 sam0x17 requested a review from KiChjang October 6, 2022 05:25
@sam0x17
Copy link
Contributor Author

sam0x17 commented Oct 6, 2022

bot rebase

@paritytech-processbot
Copy link

Rebased

@sam0x17
Copy link
Contributor Author

sam0x17 commented Oct 6, 2022

bot rebase

@paritytech-processbot
Copy link

Rebased

@sam0x17
Copy link
Contributor Author

sam0x17 commented Oct 6, 2022

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

@sam0x17
Copy link
Contributor Author

sam0x17 commented Oct 6, 2022

bot merge

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis/1026/2

ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…ty and discovery + revamp of pallet macro documentation (paritytech#12334)

* proof of concept working for pallet::whitelist_storage

* fix comments

* pallet macros docs rewrite WIP

* fix issue with cargo fmt cobbling links

* tweak capitalization

* fix docs for storage_version

* fix docs for pallet::hooks

* fix several comments

* fix invalid link

* fix wrapping and add missing links for pallet::hooks docs

* run rewrap on all text blocks in frame_support::pallet docs

* cargo fmt

* fix up pallet::call_index docs

* fix docs for pallet::extra_constants

* fix docs for pallet::error

* fix docs for pallet::event

* fix docs for pallet::event

* * fix docs for pallet::storage
* fix docs for pallet::getter
* fix docs for pallet::storage_prefix
* fix docs for pallet::unbounded
* fix docs for pallet::whitelist_storage
* fix docs for #[cfg(..)] (for storage items and attributes)
* fix docs for pallet::storage macro expansion

* fix docs for pallet::type_value

* fix docs for pallet::genesis_config

* fix docs for pallet::genesis_build

* fix docs for pallet::inherent

* fix docs for pallet::validate_unsigned

* fix docs for pallet::origin

* fix docs for general notes on instantiable pallets

* fix docs for example of a non-instantiable pallet

* fix docs for example of an instantiable pallet

* fix docs for upgrade guidelines

* fix docs for upgrade guidelines

* fix docs for upgrade checking and final notes

* fix some examples near the beginning

* extract docs for `pallet::whitelist_storage`

* add docs for pallet_macro_stub

* fix order of pallet::config and pallet::constant

* set up stub for pallet::config

* set up stub for pallet::constant

* fix

* set up stub for pallet::disable_frame_system_supertrait_check

* set up stub for pallet::generate_storage_info

* set up stub for pallet::storage_version

* set up stub for pallet::hooks

* set up stub for pallet::weight

* set up stub for pallet::compact

* set up stub for pallet::call_index

* set up stub for pallet::extra_constants

* set up stub for pallet::error

* set up stub for pallet::event

* set up stub for pallet::generate_deposit

* set up stub for pallet::storage

* set up stub for pallet::getter

* set up stub for pallet::storage_prefix

* set up stub for pallet::unbounded

* set up stub for pallet::type_value

* set up stub for pallet::genesis_config

* set up stub for pallet::genesis_build

* set up stub for pallet::inherent

* set up stub for pallet::validate_unsigned

* set up stub for pallet::origin

* fix comment

* cargo fmt

* tweak error message

* Update frame/support/procedural/src/lib.rs

Co-authored-by: Keith Yeung <[email protected]>

* Update frame/support/procedural/src/lib.rs

Co-authored-by: Keith Yeung <[email protected]>

* switch order of derives

Co-authored-by: Squirrel <[email protected]>

* tweak wording

Co-authored-by: Squirrel <[email protected]>

* add more context info about `MAX_MODULE_ERROR_ENCODED_SIZE`

Co-authored-by: Squirrel <[email protected]>

* tweak wording about where clause

Co-authored-by: Squirrel <[email protected]>

* clarify wording about system/events key

Co-authored-by: Squirrel <[email protected]>

* use "The Event enum" instead of "item"

Co-authored-by: Squirrel <[email protected]>

* fix bad wording

Co-authored-by: Squirrel <[email protected]>

* use enum instead of type

Co-authored-by: Squirrel <[email protected]>

* expect => expects

Co-authored-by: Squirrel <[email protected]>

* add additional note about storage prefix

Co-authored-by: Squirrel <[email protected]>

* clearer note about GenesisConfig

Co-authored-by: Squirrel <[email protected]>

* Use "The impl" instead of "The item"

Co-authored-by: Squirrel <[email protected]>

* add note and link to tight-coupling docs

Co-authored-by: Squirrel <[email protected]>

* cargo fmt

* remove spaces around parenthesis

* fix missing text for pallet::config

* fix issue with pallet::constant intro

* fix wording about codec index

* fix pallet::error wording

* fix comment about 1 byte => 256 errors

* fix where clause comment

* fix comment about where pallet events are stored

* rewrap some text

* fix pallet::storage docs

* fix pallet::storage_prefix docs

* tweak docs for pallet::genesis_build

* tweak docs for pallet::config

* specify that pallet::event must be present if pallet::config is present

* add note about why we would want to bypass the supertrait check

* mention that pallet::generate_store attribute is only valid on pallet struct

* add note about adding new calls to the end to maintain existing order

* add note about pallet::type_value and pallet::storage

Co-authored-by: Squirrel <[email protected]>

* add note about using pallet::type_value alongside pallet::storage

* include warning about modifying disaptchables on other pallet::call_index docs page

* fix incorrect comment

* add much more information for pallet::inherent

* move pallet::pallet macro expansion notes back to their rightful place

* re-run CI

* fix macro expansion appearing in wrong place for pallet::pallet

* replicate pallet::pallet docs on the pallet::pallet macro stub

* force CI re-run

Co-authored-by: Keith Yeung <[email protected]>
Co-authored-by: Squirrel <[email protected]>
Co-authored-by: parity-processbot <>
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. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

docs for pallet macros are hard to find because of how we parse pallet macros
6 participants