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

frame/support: Link call documentation only in prod-modes #14283

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Jun 1, 2023

After updating the Cargo.lock of Substrate in Cumulus, the doc CI step is failing with

[2023-05-31 16:40:08] error: public documentation for `set_custom_validation_head_data` links to private item `Pallet::set_custom_validation_head_data`
[2023-05-31 16:40:08]   --> test/runtime/src/test_pallet.rs:34:12
[2023-05-31 16:40:08]    |
[2023-05-31 16:40:08] 34 |     #[pallet::call]
[2023-05-31 16:40:08]    |               ^^^^
[2023-05-31 16:40:08]    |
[2023-05-31 16:40:08]    = note: the link appears in this line:
[2023-05-31 16:40:08]            
[2023-05-31 16:40:08]            See [`Pallet::set_custom_validation_head_data`].
[2023-05-31 16:40:08]                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[2023-05-31 16:40:08]    = note: this link will resolve properly if you pass `--document-private-items`
[2023-05-31 16:40:08]    = note: `-D rustdoc::private-intra-doc-links` implied by `-D warnings`
[2023-05-31 16:40:08] 
[2023-05-31 16:40:08] error: could not document `cumulus-test-runtime`

This is related to a change done in the PR: https://github.com/paritytech/substrate/pull/14101/files.

This fixes the issue by removing the documentation link to private items, only for pallets started with dev-mode.

Extracted from: paritytech/cumulus#2666.

// @paritytech/subxt-team

@lexnv lexnv added 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 labels Jun 1, 2023
@lexnv lexnv requested a review from a team June 1, 2023 12:35
@lexnv lexnv self-assigned this Jun 1, 2023
@lexnv lexnv requested a review from a team June 1, 2023 12:35
}
} else {
// For the dev-mode do not provide a documenation link as it will break the
// `cargo doc` if the pallet is private inside a test.
Copy link
Member

@ggwpez ggwpez Jun 1, 2023

Choose a reason for hiding this comment

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

Why is it trying to document a test? I think anything testing should not need to be documented anyway.
The cargo doc should not even look at this, is what i mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe after that substrate PR, we no longer propagate method.docs and we do instead provide a link to [Pallet:: method]. This link will cause the cargo doc step on the cumulus on test/runtime/src/test_pallet.rs, even tho it is declared as:

#[frame_support::pallet(dev_mode)]
pub mod pallet {
#[pallet::call]
	impl<T: Config> Pallet<T> {
		/// A test dispatchable for setting a custom head data in `validate_block`.
		#[pallet::weight(0)]
		pub fn set_custom_validation_head_data(
		...
		

@paritytech-ci paritytech-ci requested a review from a team June 1, 2023 13:49
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Yeah makes sense, sorry 🙈

I am surprised this was not identified by the dev-mode example in substrate.

@lexnv
Copy link
Contributor Author

lexnv commented Jun 1, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 286a681 into master Jun 1, 2023
@paritytech-processbot paritytech-processbot bot deleted the lexnv/fix-cumulus-docs branch June 1, 2023 16:08
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants