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

Remove badges in README.md #890

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

Philippe-Cholet
Copy link
Member

@Philippe-Cholet Philippe-Cholet commented Feb 28, 2024

I might have been overzealous on the badges here but I think it's nice, especially the MSRV one that does not need manual updates.

Rendered badges

Documentation crates.io MSRV License Build Status Code Coverage Total Downloads Contributors

Copy link
Member

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

I might be alone on this, but I'm not a fan of badges in READMEs, and would even prefer to see the two badges we currently have eliminated for several reasons.

Repos, ideally, are self-contained snapshots of the state of the project. Care should be taken to minimize references to outside data, or to ensure that outside references are highly durable. Furthermore, the job of a README is to succinctly convey the essential information for using a project — not to recreate UI elements that are already presented elsewhere. We additionally need to be mindful that whatever badges we put in our README are also mirrored on crates.io, so they should make sense both for a user who's browsing the code on GitHub, and for a user who's evaluating the crate on crates.io.

Our two current badges fail to meet this bar:

  • Whether CI is passing on the master branch is completely irrelevant to >99% users of this crate, who are instead grabbing releases from crates.io. It also doesn't maintain accuracy as you checkout older snapshots of the repo — it stubbornly always reflects what's currently happening on master.
  • The badge indicating our current release number on crates.io is irrelevant to anyone finding the crate on crates.io. It duplicates information present at the top of our Cargo.toml. It doesn't maintain accuracy as you check out older snapshots of the repo.

This same line of critique applies to most or all of the badges this PR adds:

  • MSRV: Our MSRV is a property of our codebase at particular points in time. It should be documented in a static way.
  • License: Github already renders this above the README. Crates.io also renders it prominently. We document it statically in our README, in our Cargo.toml, and in our LICENSE files. We don't need to duplicate this UI element.
  • Code Coverage: Same critique as build status.
  • Total Downloads: This isn't a property of our code base. It's also presented prominently in crates.io. We don't need to duplicate this UI element in our readme.
  • Total Contributors: Github already provides this information on the homepage of the repo. We don't need to duplicate this UI element in our readme.

@Philippe-Cholet
Copy link
Member Author

Well this is quite argumented and I can agree.

About Documentation (which replaces a sentence, same link), I guess we can say the end user does not care that the "docs are passing" as long as he has the link to it (and that it works, which is done in CI).

So remove the two badges instead?

@Philippe-Cholet Philippe-Cholet changed the title More badges in README.md Remove badges in README.md Feb 29, 2024
@Philippe-Cholet
Copy link
Member Author

@jswrenn
phimuemue liked your comment and my reponse on the "docs" badge so I assume he agrees.
I therefore removed all badges instead.
For me, it was instructive about how to create badges, and why it might not be a good idea.
You can merge or close this as you see fit.

Copy link
Member

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

Thanks!

@Philippe-Cholet
Copy link
Member Author

@jswrenn Because this is markdown only, according to your #767 (comment), you have to bypass branch protections to merge this.

@jswrenn jswrenn merged commit b513c98 into rust-itertools:master Feb 29, 2024
@Philippe-Cholet Philippe-Cholet deleted the readme-badges branch February 29, 2024 13:03
@Philippe-Cholet Philippe-Cholet added this to the next milestone Feb 29, 2024
lpewewq pushed a commit to lpewewq/itertools that referenced this pull request Aug 4, 2024
lpewewq pushed a commit to lpewewq/itertools that referenced this pull request Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants