Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

polkadot-sdk-docs: Use command_macro! #6624

Merged
merged 40 commits into from
Dec 10, 2024

Conversation

ndkazu
Copy link
Contributor

@ndkazu ndkazu commented Nov 23, 2024

Description

Understood assignment:
Initial assignment description is in #6194.
In order to Simplify the display of commands and ensure they are tested for chain spec builder's polkadot-sdk reference docs, find every occurrence of #[docify::export] where process:Command is used, and replace the use of process:Command by run_cmd! from the cmd_lib crate.

@iulianbarbu
Copy link
Contributor

Can you please use cmd_lib instead of command_macro? We already use it with chain-spec-builder and works fine.

@ndkazu
Copy link
Contributor Author

ndkazu commented Nov 26, 2024

Can you please use cmd_lib instead of command_macro? We already use it with chain-spec-builder and works fine.

@iulianbarbu, I used a slightly modified version of the macro used in chain-spec-builder, and now cmd_lib seems to work fine. However, I am still facing a BIG problem: The tests were failing in the first place: output.stdout was an empty [u8] (You can see it in the test generate_chain_spec). For now I can confirm that the tests are failing the same way after modification, but this isn't enough.

@ndkazu
Copy link
Contributor Author

ndkazu commented Nov 28, 2024

Bot fmt and label needed 👀

@iulianbarbu iulianbarbu added T11-documentation This PR/Issue is related to documentation. R0-silent Changes should not be mentioned in any release notes labels Nov 28, 2024
@ndkazu ndkazu requested a review from iulianbarbu November 29, 2024 12:52
Copy link
Contributor

@iulianbarbu iulianbarbu left a comment

Choose a reason for hiding this comment

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

Thanks @ndkazu , one small remark related to the PRDOC.

prdoc/pr_6624.prdoc Outdated Show resolved Hide resolved
@ndkazu
Copy link
Contributor Author

ndkazu commented Nov 29, 2024

@iulianbarbu it's done: Thank you for the mentoring during this PR⇒ I learned a lot!!!!

@ndkazu
Copy link
Contributor Author

ndkazu commented Dec 2, 2024

It would be nice to review/merge this PR and close the corresponding issue: @michalkucharczyk , we need your help 🙏 !!

prdoc/pr_6624.prdoc Outdated Show resolved Hide resolved
@michalkucharczyk
Copy link
Contributor

michalkucharczyk commented Dec 9, 2024

Probably due to a lack of maintenenance of the crate command_macro the following file needed some modifications: ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/command-macros-plugin-0.2.7/src/lib.rs

Is this still a problem? If not, would you please update description?

@michalkucharczyk
Copy link
Contributor

The tests using #[docify::export] & process:Command are failing to begin with (tested & worked on: ~/polkadot-sdk/docs/sdk/src/reference_docs/chain_spec_runtime/tests/chain_spec_builder_tests.rs)

Should be fixed by:
#6673

@ndkazu
Copy link
Contributor Author

ndkazu commented Dec 10, 2024

Probably due to a lack of maintenenance of the crate command_macro the following file needed some modifications: ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/command-macros-plugin-0.2.7/src/lib.rs

Is this still a problem? If not, would you please update description?

Description updated!

@iulianbarbu iulianbarbu added this pull request to the merge queue Dec 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 10, 2024
@iulianbarbu iulianbarbu added this pull request to the merge queue Dec 10, 2024
Merged via the queue into paritytech:master with commit 19bc578 Dec 10, 2024
199 of 200 checks passed
@bkchr
Copy link
Member

bkchr commented Dec 10, 2024

/tip small

Copy link

@bkchr A referendum for a small (20 DOT) tip was successfully submitted for @ndkazu (12TDnzaKZWVLjZfztYV1Ty49CZAj99pAZPM6ovnsVCcqHbb on polkadot).

Referendum number: 1344.
tip

Copy link

The referendum has appeared on Polkassembly.

Ank4n pushed a commit that referenced this pull request Dec 15, 2024
# Description

**Understood assignment:**
Initial assignment description is in #6194.
In order to Simplify the display of commands and ensure they are tested
for chain spec builder's `polkadot-sdk` reference docs, find every
occurrence of `#[docify::export]` where `process:Command` is used, and
replace the use of `process:Command` by `run_cmd!` from the `cmd_lib
crate`.

---------

Co-authored-by: Iulian Barbu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T11-documentation This PR/Issue is related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants