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

[fix lint warnings: Uniques pallet] fix clippy::missing_docs_in_private_items warnings #14591

Merged
merged 3 commits into from
Jul 24, 2023

Conversation

sacha-l
Copy link
Contributor

@sacha-l sacha-l commented Jul 17, 2023

This PR fixes 6 out of the 8 lint warnings emitted by the missing_docs_in_private_items lint in the Uniques pallet by adding docs to the relevant types. The remaining 2 warnings have to do with the outer macro pattern I believe and may need to be addressed as a separate task (cc @sam0x17):

warning: missing documentation for a module
   --> frame/uniques/src/lib.rs:441:12
    |
441 |     #[pallet::call]
    |               ^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
    = note: requested on the command line with `-W clippy::missing-docs-in-private-items`

warning: missing documentation for an associated function
   --> frame/uniques/src/lib.rs:280:29
    |
280 |     #[pallet::generate_deposit(pub(super) fn deposit_event)]
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items

This is part of an ongoing effort from the https://github.com/orgs/paritytech/teams/devrel team to improve developer experience.

@sacha-l sacha-l added A0-please_review Pull request needs code review. I6-documentation Documentation needs fixing, improving or augmenting. B0-silent Changes should not be mentioned in any release notes 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 labels Jul 17, 2023
@sacha-l sacha-l requested a review from jsidorenko as a code owner July 17, 2023 15:28
@sacha-l sacha-l requested a review from a team July 17, 2023 15:28
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Why do we need docs on private things?

@aaronbassett
Copy link
Contributor

Why do we need docs on private things?

Adding docs to public items should be our priority, but we should also try and document the private items because we frequently tell folks to "read the code" when it comes to learning Substrate.

If we were only directing people to the rustdocs then completely agree that we would only need to document those elements which make up the public interface—although having the private items documented certainly wouldn't hurt us—but as we're explicitly directing people to read the code too, then we really should try to ensure that all of the code is documented.

@sacha-l
Copy link
Contributor Author

sacha-l commented Jul 19, 2023

bot merge

@paritytech-processbot
Copy link

Error: Statuses failed for 176f684

@sam0x17
Copy link
Contributor

sam0x17 commented Jul 20, 2023

Why do we need docs on private things?

tooltips in RA when people are poking around. I actually document all my private items across the board in my crates, for my own sanity.

@sacha-l
Copy link
Contributor Author

sacha-l commented Jul 21, 2023

bot merge

@paritytech-processbot
Copy link

Error: Statuses failed for 176f684

@sacha-l
Copy link
Contributor Author

sacha-l commented Jul 21, 2023

@sam0x17 - can you ask the bot nicely to merge ? I don't have authorization 🙈

@sam0x17
Copy link
Contributor

sam0x17 commented Jul 21, 2023

@sam0x17 - can you ask the bot nicely to merge ? I don't have authorization see_no_evil

looks like cumulus and zombienet are still failing

@jsidorenko
Copy link
Contributor

bot merge

@paritytech-processbot
Copy link

Error: Statuses failed for 176f684

@ggwpez
Copy link
Member

ggwpez commented Jul 24, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@ggwpez
Copy link
Member

ggwpez commented Jul 24, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 052aac1 into master Jul 24, 2023
4 checks passed
@paritytech-processbot paritytech-processbot bot deleted the sl/fix-missing-docs-uniques branch July 24, 2023 13:07
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. B0-silent Changes should not be mentioned in any release notes 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 I6-documentation Documentation needs fixing, improving or augmenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants