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

update genesis_config related docs and tests and error messages #1642

Merged
merged 6 commits into from
Sep 22, 2023

Conversation

kianenigma
Copy link
Contributor

Follow-up to paritytech/substrate#14306.

I hope this also showcases the important message of: It is really not that hard to make the examples codes in rust-docs compile, and therefore remain correct. Please embrace this :)

It moves the documentation of proc macros to their re-export, such that can link other items in frame-support. This is a patter that we should embrace for all of macro docs, and apply in PRs like paritytech/substrate#13987 as well.

@kianenigma kianenigma requested review from a team September 20, 2023 10:06
@kianenigma kianenigma added the T11-documentation This PR/Issue is related to documentation. label Sep 20, 2023
@kianenigma kianenigma changed the title update genesis_config related docs and tests and error messages update genesis_config related docs and tests and error messages Sep 20, 2023
substrate/frame/support/src/lib.rs Outdated Show resolved Hide resolved
kianenigma and others added 2 commits September 20, 2023 13:24
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
@sam0x17
Copy link
Contributor

sam0x17 commented Sep 20, 2023

This is fine, but keep in mind if doing this technique, unless something has changed since the last time I checked, rust analyzer will not pick up on the re-export docs when hovering over those items, so it might make sense to put the "main docs" for the thing on the actual item, and the "examples" on the re-export. I think rustdoc will put one before the other and I forget which order but worth playing around with to see if we can get some docs in RA and all the docs in everything else instead of no docs in RA because that defeats the purpose of the stubs.

This was the reason for the documented macro stubs that I added

@kianenigma
Copy link
Contributor Author

kianenigma commented Sep 20, 2023

I think rustdoc will put one before the other and I forget which order but worth playing around with to see if we can get some docs in RA and all the docs in everything else instead of no docs in RA because that defeats the purpose of the stubs.

The one on the re-export goes first, so putting the example first is a weird experience..

Thanks for pointing this out. I would have to find another alternative. This is still a pretty much unsolved issue. Why is it that RA will get the stubs first, and not the re-exports?

UPDATE:

if we target having two umbrella crates, it might be solve-able, I think you also hinted at this at some point.

  • frame exports all types
  • frame-macros exports all macros.
  • frame-macros relies on frame
  • user have to bring in frame and frame-macros, so there would be no re-exports.

@ggwpez
Copy link
Member

ggwpez commented Sep 20, 2023

bot update-ui substrate --rust_version=1.70

@command-bot
Copy link

command-bot bot commented Sep 20, 2023

@ggwpez https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3752654 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/update-ui/update-ui.sh" --target_path=./substrate --rust_version=1.70. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 6-13bc1723-4451-41f5-bddd-fea4c9e29036 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 20, 2023

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/update-ui/update-ui.sh" --target_path=./substrate --rust_version=1.70 has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3752654 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3752654/artifacts/download.

Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

Love functioning doc tests 😍

@paritytech paritytech deleted a comment from command-bot bot Sep 20, 2023
@ggwpez ggwpez merged commit b09ab37 into master Sep 22, 2023
10 checks passed
@ggwpez ggwpez deleted the kiz-genesis-config-docs branch September 22, 2023 11:48
@kianenigma
Copy link
Contributor Author

I was not done lol :D

@kianenigma kianenigma restored the kiz-genesis-config-docs branch September 25, 2023 06:45
@kianenigma
Copy link
Contributor Author

In retrospect, this was part of #247

bkchr pushed a commit that referenced this pull request Sep 25, 2023
#1689)

Small tweak to #1642 to
incorporate the ideas from
#247.

I think this is the good middle ground, where we have good rust-docs,
and the RA users will also have some hope.

cc @wentelteefje @aaronbassett @sam0x17
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
…ritytech#1642)

Follow-up to paritytech/substrate#14306.

I hope this also showcases the important message of: **It is really not
that hard to make the examples codes in rust-docs compile, and therefore
remain correct. Please embrace this :)**

It moves the documentation of proc macros to their re-export, such that
can link other items in frame-support. This is a patter that we should
embrace for all of macro docs, and apply in PRs like
paritytech/substrate#13987 as well.

---------

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: command-bot <>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
paritytech#1689)

Small tweak to paritytech#1642 to
incorporate the ideas from
paritytech#247.

I think this is the good middle ground, where we have good rust-docs,
and the RA users will also have some hope.

cc @wentelteefje @aaronbassett @sam0x17
bkchr pushed a commit that referenced this pull request Apr 10, 2024
* remove message fee

* it is compiling!

* fixes + fmt

* more cleanup

* more cleanup

* restore MessageDeliveryAndDispatchPayment since we'll need relayer rewards

* started rational relayer removal

* more removal

* removed estimate fee subcommand

* remove DispatchFeePayment

* more removals

* removed conversion rates && some metrics

* - unneeded associated type

* - OutboundMessageFee

* fix benchmarks compilation

* fmt

* test + fix benchmarks

* fix send message

* clippy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T11-documentation This PR/Issue is related to documentation.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

10 participants