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

Readme improvements #6287

Merged
merged 1 commit into from
Nov 7, 2020
Merged

Readme improvements #6287

merged 1 commit into from
Nov 7, 2020

Conversation

camsteffen
Copy link
Contributor

@camsteffen camsteffen commented Nov 3, 2020

Moved the table of contents up, added an Overview heading.

Made the "All the Clippy Lints" link clearer.

Formatted the lint categories as a table with the default lint level (instead of saying on/off by default). Tweaked the descriptions.

changelog: Improve Readme

@rust-highfive
Copy link

r? @llogiq

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 3, 2020
@flip1995
Copy link
Member

flip1995 commented Nov 3, 2020

I don't see how this makes the "All the Clippy lints" link clearer. Having the ToC where it currently is, is actually intentional, since we want to have the most relevant information for the user at the top. If you want more information, you scroll past this either way. Having a ToC on top doesn't help find this information faster.

-1 for this change from me.

I like the reformat to a table though (but without the traffic lights). clippy::all is warn/deny

@llogiq
Copy link
Contributor

llogiq commented Nov 3, 2020

As @flip1995 wrote, we would want some changes before merging this.

I will look into this later, just know that we'll need some discussion.

@camsteffen
Copy link
Contributor Author

camsteffen commented Nov 3, 2020

Here is take 2 with flip1995's input: camsteffen@25be6e7

@flip1995
Copy link
Member

flip1995 commented Nov 4, 2020

The changes in camsteffen@25be6e7 LGTM. @llogiq r=me if those changes are pushed and you're also good with them. 👍

@llogiq
Copy link
Contributor

llogiq commented Nov 4, 2020

The problem becomes that cargo dev update_lints expects a number of certain sequences of text in the README, which have now changed. Look at clippy_dev/src/update_lints.rs.

Format lint categories as a table with the default lint level.
@camsteffen
Copy link
Contributor Author

Pushed the revised version. I think that resolves the concern with cargo dev update_lints. The main thing is adding default lint levels so I don't mind tossing the other changes.

@llogiq
Copy link
Contributor

llogiq commented Nov 7, 2020

Thank you! @bors r+

@bors
Copy link
Contributor

bors commented Nov 7, 2020

📌 Commit fd8dece has been approved by llogiq

@bors
Copy link
Contributor

bors commented Nov 7, 2020

⌛ Testing commit fd8dece with merge 4aca13f...

@bors
Copy link
Contributor

bors commented Nov 7, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 4aca13f to master...

@bors bors merged commit 4aca13f into rust-lang:master Nov 7, 2020
@camsteffen camsteffen deleted the readme branch July 8, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants