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

Improve the appearance of crates on crates.io #5243

Merged
merged 6 commits into from
Aug 23, 2024
Merged

Conversation

rzadp
Copy link
Contributor

@rzadp rzadp commented Aug 5, 2024

Context

Currently, many crates have no readme files, even though they are public and available on crates.io.
Others have just a couple of words that does not look super presentable.

Even though probably nobody starts a journey with polkadot-sdk or its documentation with a readme of a random low-level crate, I think it would look more mature to have a little better readmes there.

So, in an attempt to improve the aesthetics of polkadot-sdk, I propose a set of consistent, branded, better-looking readmes for all published crates.

What's inside

  • New readme files for published crates.
  • A python script to generate new readmes, for the crates that have none.
    • It will skip crates that do have a readme, and private crates.
  • Added a new image asset to the repo - logo with a background.
    • The main readme of the repo uses a nice trick to accompany both light and dark modes - but that doesn't work on crates.io so a single logo with a background is needed.

Example

Current

Screenshot 2024-08-04 at 16 13 36

Changed

Screenshot 2024-08-04 at 16 12 28

@rzadp rzadp added R0-silent Changes should not be mentioned in any release notes T11-documentation This PR/Issue is related to documentation. labels Aug 5, 2024
@rzadp rzadp requested a review from kianenigma August 5, 2024 13:34
@kianenigma
Copy link
Contributor

kianenigma commented Aug 5, 2024

meta: I think this type of work is great! Thanks for taking it on.

About the actual details:

With consistent format, all readmes can be updated via search&replace tools.

I wish there was a standard template file + a python script that would do it? For example, imagine we want to change the image, or add a new line to all READMEs. The manual search and replace indeed works, but I think nowadays with AI-assisted coding it is worthwhile to script it in py or js.

I imagine:

  1. .template file that has a few fields like:
  • crate description (coming from cargo.toml)
  • crate public doc URL (hardcoded and generated based on the crate name)
  1. Prior to each release, the script is triggered, and the new READMEs are pushed to master.

By the way, this closes #1260.

I think it is also great that you are focusing on building a system here, rather than fixing e.g. crate descriptions. End of the day, only devs can provide good one-liner description of what a crate is, and we can hope they do that in the future. But you or I are not the right people to retroactively try and change these descriptions.

As noted in the original Aesthetics issue, we should pay closer attention to the README of only a few creates, and myself and or @ggwpez @gupnik will eventually do so, if it makes sense:

  • polkadot-sdk
  • polkadot-sdk-frame
  • frame-system
  • frame-support
  • polkadot
  • polkadot-parachain

Perhaps for these we can have a special README that is not auto-generated.

@kianenigma
Copy link
Contributor

Actually, actually, as the part of the same python script that is ran upon new release, we can also solve https://github.com/paritytech/release-engineering/issues/196, which IMO is an important issue. Thoughts @ggwpez @EgorPopelyaev?


# Cumulus-Specific Common Consensus Implementations

This crate is part of the [Polkadot SDK](https://github.com/paritytech/polkadot-sdk/).
Copy link
Member

Choose a reason for hiding this comment

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

How where those generated? Looks good IMHO, just asking.
Eventually we should add a CI that ensures that they stay in sync with the rust files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How where those generated? Looks good IMHO, just asking.

I did some one-off local bash scripting to generate the initial files, but then I relied on manual work to finish them, or to adapt already-existing readmes.

I guess it can be fully-automatic if:

  • We accept the Cargo.toml's name as a title.
    • For example, crate pallet-nft-fractionalization can have an automatic title like Pallet Nft Fractionalization. But not a nicer NFT Fractionalization Pallet as I have handcrafted.
  • The Cargo.toml's description is taken at face value, not rewritten into another sentence, etc.
    Missing capitalization or a dot at the end can be auto-added.
  • We leave a pack of handcrafted readmes untouched by the auto-generation script, as mentioned by @kianenigma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CI that ensures that they stay in sync with the rust files.

What do you mean exactly? In my mind, those readmes were never supposed to be out-of-sync with the rust files.

For example, this crate cumulus-client-consensus-common will always have a link to docs (...)/cumulus_client_consensus_common, and it will never contain anything other than "Cumulus-specific common consensus implementations" - at least I hope not :D

Copy link
Member

Choose a reason for hiding this comment

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

I just wondered if you used something like this https://crates.io/crates/cargo-readme to generate the READMEs from the rust docs, since that can be put into CI to have up-to-date READMEs.
Maybe i misunderstood what you are trying to do here?

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 was thinking the READMEs would be a small shell with a small intro, that invites the user to go to rendered rustdocs to consume the docs - not a copy of the rustdocs text converted to markdown.

Copy link
Member

Choose a reason for hiding this comment

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

I dont really mind, but i personally understood the "everything in rust docs" maxime as having the source of truth in rust docs, and then making it more pleasing to the Eye by auto generating different formats. Eg READMEs or Wiki entries.

Copy link
Contributor

Choose a reason for hiding this comment

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

The main issue about generating README from .rs files is that they would not be able to parse docify stuff.

Based on https://github.com/sam0x17/docify, there is a way to do this, but it would require first generating markdown files from .rs files, then tweaking the docify syntax, then running another process around it etc.

I recall talking with @sam0x17 last year about a CLI, but we never did it.

In general, it is a cool idea, but not sure if it is a good bang for our buck atm. I would be happy with an auto-generated, mostly empty .md file that points to the right rustdoc page. It is less pretty, but works as good.

@ggwpez
Copy link
Member

ggwpez commented Aug 5, 2024

Actually, actually, as the part of the same python script that is ran upon new release, we can also solve paritytech/release-engineering#196, which IMO is an important issue.

We would have to do these changes just before releasing. @Morganamilo i think its a good idea to append the SDK version to the README as well (additionally to the crate description).

@rzadp
Copy link
Contributor Author

rzadp commented Aug 6, 2024

I wish there was a standard template file + a python script that would do it? For example, imagine we want to change the image, or add a new line to all READMEs. The manual search and replace indeed works, but I think nowadays with AI-assisted coding it is worthwhile to script it in py or js.

I was thinking that by adding those minimal readmes without a script, I could avoid adding a thing that could go wrong to the release process - even if it's a small thing. But perhaps I was overthinking.


Perhaps for these we can have a special README that is not auto-generated.

Yes, we can have that. I was operating under the impression that we don't want that, and have everything in .rs. But I guess there's always exceptions :D


I'll rewrite the PR to have an automatic py script with the following plan:

  1. Delete readmes that only duplicate information from Cargo.toml - like this.
  2. Add a readme template file.
  3. For every (published) crate:
  • If a readme file exists, skip - meaning it's one of the handcrafted ones.
  • Fill the template with information based on Cargo.toml.
  1. The script could either take SDK version as a parameter and be used just before releasing, or it could be used to update files on master and the release process would append the version separatelly.

@rzadp rzadp changed the title Improve the appearance of Substrate and Cumulus on crates.io Improve the appearance of crates on crates.io Aug 12, 2024
@rzadp
Copy link
Contributor Author

rzadp commented Aug 12, 2024

I have recreated the PR with a different approach.

It now only contains a python script that generates a readme for each public crate - based on the name, manifest description, license, and (optionally) SDK release version.

The script will omit crates that already have a readme - that way we can have special, handcrafted readmes.

It can be included in the release process as a non-critical step.

Example

Link to documentation

Initially, I thought it was favorable to point the user to consume rustdocs about a crate on the Polkadot SDK docs site - for example: https://paritytech.github.io/polkadot-sdk/master/sc_consensus_beefy_rpc/

I now think the opposite, because:

  • Polkadot SDK docs don't contain versioned docs for single crates - we can only point to master, but that can contain docs for a different version of a crate that the user is interested in.
  • Crates.io already contain a docs.rs link with a proper version.
  • Some crates don't yet have a proper documentation - it creates a false promise, generating a link that doesn't give you much when you follow it.

So, I'm not generating a link like that anymore.

Special crates

As @kianenigma you said, there are special crates, such as polkadot-sdk or polkadot, that have the most visibility and should be treated specially.

I thought about handcrafting readmes those, but I didn't have a good vision for that, so I settled for a few fixes in the one that existed.

@rzadp rzadp marked this pull request as ready for review August 12, 2024 15:34
@rzadp rzadp requested review from a team as code owners August 12, 2024 15:34
VERSION_TEMPLATE = """
## Version

This version of `{name}` is associated with Polkadot {sdk_version} release.
Copy link
Contributor

Choose a reason for hiding this comment

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

The sdk release names will change to the new format highlighted by Oliver in https://forum.parity.io/t/new-release-process-brainstorming-on-improvements/2296/8

But it should be fine, just so you know.

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.

Your opinions in the last iteration make sense to me. Let's keep this iterative.

You can explore how hard is it for the release team to update these in the first stable release, that woudl be cool.

This makes me again try and get polkadot-sdk-docs to publish to crates.io, it will then solve a lot of our issues. The reason why have paritytech.github.io now is that we cannot publish to crates.

@rzadp rzadp added this pull request to the merge queue Aug 23, 2024
Merged via the queue into master with commit c66f7bd Aug 23, 2024
185 of 188 checks passed
@rzadp rzadp deleted the rzadp/crate-readmes branch August 23, 2024 11:26
@kianenigma
Copy link
Contributor

Super nice, looking forward to nice looking stuff in docs.rs and crates.io in our next release!

ordian added a commit that referenced this pull request Aug 27, 2024
* master: (36 commits)
  Bump the ci_dependencies group across 1 directory with 2 updates (#5401)
  Remove deprecated calls in cumulus-parachain-system (#5439)
  Make the PR template a default for new PRs (#5462)
  Only log the propagating transactions when they are not empty (#5424)
  [CI] Fix SemVer check base commit (#5361)
  Sync status refactoring (#5450)
  Add build options to the srtool build step (#4956)
  `MaybeConsideration` extension trait for `Consideration` (#5384)
  Skip slot before creating inherent data providers during major sync (#5344)
  Add symlinks for code of conduct and contribution guidelines (#5447)
  pallet-collator-selection: correctly register weight in `new_session` (#5430)
  Derive `Clone` on `EncodableOpaqueLeaf` (#5442)
  Moving `Find FAIL-CI` check to GHA (#5377)
  Remove panic, as proof is invalid. (#5427)
  Reactive syncing metrics (#5410)
  [bridges] Prune messages from confirmation tx body, not from the on_idle (#5006)
  Change the chain to Rococo in the parachain template Zombienet config (#5279)
  Improve the appearance of crates on `crates.io` (#5243)
  Add initial version of `pallet_revive` (#5293)
  Update OpenZeppelin template documentation (#5398)
  ...
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