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

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

ndkazu
Copy link
Contributor

@ndkazu ndkazu commented Nov 23, 2024

Description

Understood assignment:
Initial assignment description is in #6194 : find every occurrence of #[docify::export] where process:Command is used → replace the use of process:Command by command_macro!

Issues observed:

  • 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)
  • 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

I am running cargo +nightly test from ~/polkadot-sdk/docs/sdk/src/reference_docs/chain_spec_runtime/

Same test failures are observed after changing from process:Command to command_macro!

Some assistance is needed.

@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
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.

This is almost there I think, thanks!

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!!!!

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.

2 participants