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.

Sign up for GitHub

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

Jump to bottom

Update lint documentation to use markdown headlines #7420

Merged
merged 2 commits into from
Jul 28, 2021
Merged

Update lint documentation to use markdown headlines #7420

merged 2 commits into from
Jul 28, 2021

Conversation

xFrednet
Copy link
Member

@xFrednet xFrednet commented Jul 2, 2021 •
edited
Loading

This PR updates all lint documentation to use markdown headlines. It additionally removed the Known problems section for lints without any problems. I've double-checked all automatic replacements, but a second pair of eyes is definitely appreciated!

I wasn't sure when you wanted to switch to the new metadata collection tomorrow, I therefore prepared this PR today. And that's it this is a standalone PR to keep the other related PRs reviewable.

changelog: none

r? @flip1995

cc: #7172

Note: This should be merged with the other metadata collection related PRs.

@xFrednet
Copy link
Member Author

xFrednet commented Jul 2, 2021 •
edited
Loading

r? @flip1995

seems like r? isn't working currently 🤔

@flip1995 flip1995 self-assigned this Jul 5, 2021
@flip1995
Copy link
Member

flip1995 commented Jul 5, 2021

seems like r? isn't working currently thinking

Yeah, it seems GitHub recently has problems with webhooks, so the bots are a bit flaky.

As you may have noticed: I didn't get to the move this weekend and try to fit it in during this week.

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 14, 2021
@bors
Copy link
Contributor

bors commented Jul 14, 2021

☔ The latest upstream changes (presumably #7462) made this pull request unmergeable. Please resolve the merge conflicts.

@xFrednet
Copy link
Member Author

I'm actually surprised it took this long xD. I'll update the PR in the next couple of days (hopefully tomorrow)

@xFrednet xFrednet force-pushed the 7172-update-lint-documentation branch from 3a25292 to 2d26817 Compare July 15, 2021 13:29
@camsteffen
Copy link
Contributor

self_named_constructor lint was added

@xFrednet
Copy link
Member Author

Thank you for the ping! I'll update this PR tomorrow 🙃

@xFrednet xFrednet force-pushed the 7172-update-lint-documentation branch from 2d26817 to bb9cdea Compare July 23, 2021 11:02
@bors
Copy link
Contributor

bors commented Jul 24, 2021

☔ The latest upstream changes (presumably #7482) made this pull request unmergeable. Please resolve the merge conflicts.

@xFrednet xFrednet force-pushed the 7172-update-lint-documentation branch from bb9cdea to 5eeff3c Compare July 24, 2021 20:50
@bors
Copy link
Contributor

bors commented Jul 27, 2021

☔ The latest upstream changes (presumably #7488) made this pull request unmergeable. Please resolve the merge conflicts.

@xFrednet xFrednet force-pushed the 7172-update-lint-documentation branch from 5eeff3c to ffa8de9 Compare July 27, 2021 16:30
@xFrednet xFrednet mentioned this pull request Jul 28, 2021
Merged
1 task
flip1995
flip1995 approved these changes Jul 28, 2021
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM overall. This messes up the formatting of the in-code documentation a bit (line length of the first doc line) but we weren't consistent in that regard anyway. Also it won't have any influence on the website.

I'll merge this after the move.

doc/adding_lints.md
@@ -11,6 +11,7 @@ because that's clearly a non-descriptive name.
- [Setup](#setup)
- [Getting Started](#getting-started)
- [Testing](#testing)
- [Cargo lints](#cargo-lints)
Copy link
Member

Choose a reason for hiding this comment

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

Ninja fix 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

xD even for me. A IDE plugin did that and I only noticed it when I reviewed the changes on GitHub xD

I would also suggest merging this last, another idea I had was to crate a rollup PR that combines all metadata related PRs 🤔 You'll find a good solution, I have faith in you 🙃

@flip1995
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 28, 2021

📌 Commit 906464e has been approved by flip1995

@bors
Copy link
Contributor

bors commented Jul 28, 2021

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Jul 28, 2021
@flip1995
Rollup merge of rust-lang#7420 - xFrednet:7172-update-lint-documentat…
e82d0d3 
…ion, r=flip1995

Update lint documentation to use markdown headlines

This PR updates all lint documentation to use markdown headlines. It additionally removed the *Known problems* section for lints without any problems. I've double-checked all automatic replacements, but a second pair of eyes is definitely appreciated!

I wasn't sure when you wanted to switch to the new metadata collection tomorrow, I therefore prepared this PR today. And that's it this is a standalone PR to keep the other related PRs reviewable.

changelog:  none

r? `@flip1995`

cc: rust-lang#7172

Note: This should be merged with the other metadata collection related PRs.
@flip1995 flip1995 mentioned this pull request Jul 28, 2021
Closed
@flip1995 flip1995 force-pushed the 7172-update-lint-documentation branch from 906464e to 9bc5803 Compare July 28, 2021 12:32
@flip1995
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 28, 2021

📌 Commit 9bc5803 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Jul 28, 2021

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@flip1995 flip1995 mentioned this pull request Jul 28, 2021
Merged
bors added a commit that referenced this pull request Jul 28, 2021
@bors
Auto merge of #7502 - flip1995:rollup-y3ho3w0, r=flip1995
92ca25b 
Rollup of 3 pull requests

Successful merges:

 - #7279 (Adapting the lint list to Clippy's new metadata format)
 - #7298 (Switch CI to new metadata collection)
 - #7420 (Update lint documentation to use markdown headlines)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

changelog: rollup
@bors bors merged commit 464c85c into rust-lang:master Jul 28, 2021
@xFrednet xFrednet deleted the 7172-update-lint-documentation branch July 28, 2021 16:44
bors added a commit that referenced this pull request Jul 29, 2021
@bors
Auto merge of #7498 - xFrednet:changelog-1-55, r=flip1995
b3b2024 
Updated changelog for 1.55

This has again been a bit of work, but I'm happy to notice that my English is still improving, and I'm getting faster at these things. That's a very nice side effect of contributing and getting feedback on reviews 😊

Moving on, there were a few things that I was unsure about:
* The PR rust-lang/rust#86717 changes an old entry in the change log, is this worth mentioning? I've left it out for now.
* The stabilization of `cargo clippy --fix` is quite awesome and important IMO. It sadly gets a bit lost in the *Other* entry, as it's the last one. Do we maybe want to move it somewhere else or change the headline order for this release?
* I've listed the introduction of the new `suspicious` group under the *Moves and Deprecations* section. Is this alright, or should it be moved to the *Other* section as well?
* Last but definitely not least, some fun! I've used the 🎉 emoji in the `cargo clippy --fix` entry, is this okay?

Sorry for the bombardment of questions xD

---

The PR already includes the entries for the new metadata collection and website updates. These are not merged yet, but should probably be to make this correct. This might also require the commit hashes to be updated (Not sure on this, though). It would actually be super fitting to get this into this release as we also stabilize `--fix`. TODOs:
* [x] Merge metadata collection PRs:
  1. #7279
  2. #7298
  3. #7420 (Hope to not get any merge conflicts)

---

[Rendered 📰](https://github.com/xFrednet/rust-clippy/blob/changelog-1-55/CHANGELOG.md)

r? `@flip1995`

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 approved these changes

Assignees

@flip1995 flip1995

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xFrednet @flip1995 @bors @camsteffen