rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.46k stars 1.54k forks source link

[Tracking issue] Project: Clippy Book Chapter Updates Reborn #10597

Open blyxyas opened 1 year ago

blyxyas commented 1 year ago

Tracking issue for all relating to the Clippy Book Chapter Updates Reborn project. If you have anything relating to the whole project, please comment it here instead of in an individual PR.

NAME STATUS REF
Defining lints Merged #10595
Writing tests Merged #10596
Lint passes Merged #10622
Emitting lints Merged #10598
Type Checking Merged #10605
Trait Checking PR Open 🔮 #10626
Method Checking Merged #10644
Macro Expansions Merged #10652
HirIds Needs discussion 🐝
Example: Late Lint Working on 🦕
Example: Early Lint PR Open 🔮 #11352
Adding documentation Not started yet 🍅
The review process Not started yet 🍅

You can comment about which chapters to add or remove.

Ideas

Discussions

For more detailed answer about these (or new) discussions, you can also create a comment and I'll add it here.


A lot of the discussion about this project was done through the draft PR #9426, but please, use this tracking issue instead.

blyxyas commented 1 year ago

@rustbot label C-tracking-issue A-documentation

xFrednet commented 1 year ago

Should these new updates replace the current "Adding lints" chapter?

Yes, in my opinion they should. The current "Adding lints" grew over time and is now too long and not well-structured. There is no real separation between that and the "Common Tools" section. Reworking that would be a gigantic help

Should they use an storytelling / informal style to welcome new users?

This depends on the part of the documentation. I like the storytelling style, but I know that I'm sometimes less formal than others in this project. Here I would put trust in the reviewer :)

Lint passes: Re-write the current section (here) or create a whole new chapter?

Rewrite! The other PR already made some good progress on this. I would also like to put a focus on the LateLintPass as most lints should use that one :)


Side note: I think the voting with emojis will not work too well. Requesting for text answers or asking in the next meeting might work better. :)

We sadly can't check every created issue due to the number of notifications, so it's likely that this issue has been missed by most. You can try to ping the Clippy team @rust-lang/clippy to get more feedback. A small note about this as well: Even if you ping us all, it's likely that you only get two or three responses. Don't let that discourage you, this usually means that the others agree with what has been said, don't have strong opinions on the matter, or in some cases, didn't have time to give detailed feedback. (It took me some time to learn this when I stated contribution. That's why I'm pointing that out early :))


Btw, thank you for the issue and the detailed table :)

blyxyas commented 1 year ago

Yeah voting with emojis won't go well, I noticed as soon as I wanted to create a fourth discussion 😅

blyxyas commented 1 year ago

Lint passes: Re-write the current section (here) or create a whole new chapter?

Re-write! [...]

I found a problem with re-writing the current section on "Adding lints", if we're going to replace "Adding lints" with these new chapters, there won't really be an "Adding lints".

If Adding lints is going to be gone, creating a new chapter is quite literally the only way to go.

blyxyas commented 1 year ago

I think now is a pretty good moment to say that English isn't my first language... Such a good idea to take revamping the documentation as a project...

xFrednet commented 1 year ago

No worries, I believe that only one or two members have English as their mother tong. The reviews should catch simple mistakes :). If it's about spelling, I can highly recommend languagetool.org and the browser add-on. My spelling is... not perfect, but I'm fairly confident with that tool. I sadly don't know of a good IDE integration.

So please don't let this stop you!!

xFrednet commented 1 year ago

I think it would be cool, to also include a reviewing checklist as a general guideline. I have a list of things I usually look out for, but they're not really documented anywhere. Here is a list of things I can think of rn:

There are probably a lot of others as well, but these are the ones that quickly came to mind

blyxyas commented 1 year ago

I was working on the Hir Ids chapter and as @flip1995 said here, it doesn't seem to be really necessary (?). The first 3 - 4 paragraphs are useful, but they'd probably fit elsewhere :cow:

Centri3 commented 1 year ago

Maybe we can mention the parent_module, module_children, module_children_local, impl_parent and associated_item queries? Basically navigating the DefId map? Chapters already mention how to do this on the HIR but not for non-local DefIds which come up sort of often

(We also need a utils function for this...)

For example, #11139 needed this so we didn't hardcode it for std types.