Skip to content

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

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

Open
blyxyas opened this issue Apr 5, 2023 · 9 comments
Open

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

blyxyas opened this issue Apr 5, 2023 · 9 comments
Labels
A-documentation Area: Adding or improving documentation C-tracking-issue Category: Tracking Issue

Comments

@blyxyas
Copy link
Member

blyxyas commented Apr 5, 2023

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

  • "Tips from collaborators / experts" section / chapter?

Discussions

  • Should these new updates replace the current "Adding lints" chapter?
  • Should they use an storytelling / informal style to welcome new users?
  • Lint passes: Re-write the current section (here) or create a whole new chapter? For this one take into account that this project is basically de-structuring the "Adding Lints" chapter into a lot of smaller ones, so creating a new chapter isn't a big deal.

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
Copy link
Member Author

blyxyas commented Apr 5, 2023

@rustbot label C-tracking-issue A-documentation

@rustbot rustbot added A-documentation Area: Adding or improving documentation C-tracking-issue Category: Tracking Issue labels Apr 5, 2023
@xFrednet
Copy link
Member

xFrednet commented Apr 5, 2023

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
Copy link
Member Author

blyxyas commented Apr 5, 2023

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

@blyxyas
Copy link
Member Author

blyxyas commented Apr 7, 2023

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.

bors added a commit that referenced this issue Apr 8, 2023
Clippy Book Chapter Updates Reborn: Type Checking

This PR adds a new chapter to the book: "Type Checking", it hasn't changed a lot from the source mainly because there wasn't many reviews on it and I haven't see a lot of things that needed a change.

## Notes

- I have some doubts about the whole "`is_*` Usage" section, what do you think about it.
- For discussion about  the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions, and more information)

changelog: Add a new "Type Checking" chapter to the book

r? `@flip1995`
@blyxyas
Copy link
Member Author

blyxyas commented Apr 10, 2023

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
Copy link
Member

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!!

bors added a commit that referenced this issue Apr 14, 2023
Clippy Book Chapter Updates Reborn: Lint Passes

This PR adds a new chapter to the book: "Lint passes". No major changes apart from some re-phrasing, fixing typos... etc.

## Notes

- Requires #10595 to be merged before this one (Or else, a link will be broken).
- To talk about the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions and more information)

changelog: Add a new "Lint passes" chapter to the book
r? `@flip1995`
bors added a commit that referenced this issue Apr 16, 2023
Clippy Book Chapter Updates Reborn: Macro Expansions

This PR adds a new chapter to the book: "Macro Expansions". There weren't big changes apart from grammar, re-phrasing and stylistic choices.

## Notes

- **Does not require any other chapter** to be merged before this
- To talk about the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions and more information)

changelog: Add a new "Macro Expansions" chapter to the book

r? `@flip1995`
bors added a commit that referenced this issue May 20, 2023
Update URLs in Type Checking chapter

Updated links to `Adt`, `pat_ty` and `Tykind` in the "Type Checking" chapter of the book.

### Notes:
- For discussion about the whole project, please use the tracking issue for the project #10597  (It also contains a timeline, discussions, and more information).

changelog: Fix broken links in "Type Checking" chapter of the book
bors added a commit that referenced this issue Jun 2, 2023
Clippy Book Chapter Updates Reborn: Refresh Lint Configuration's looks

This PR modernizes and clears up some confusion with the "Lint Configuration Options" chapter from the book.

### Changes

- **Remove 'Option - Default Value" table**

    - Why was it even there?
    - It shouldn't be the first thing an user sees when they enter the chapter. It's clunky, ugly and not useful. The default values for configs are stated in a per-config basis if needed.

- **Add a simple description of what the chapter contains, and the scheme of each configuration option**

- **Minor formatting, mainly adding code fragments to code text**

    - It seemed weird and jarring not having back-ticks on text like "arithmetic_side_effects".
    -  Improves readability and separation between configs.

- **Separate a little bit the Affected Lints list + "Affected lists" message**

    - Not having something indicating that the list is about the lints that use the configuration option is confusing.
    - It isn't as important as the description and example. Therefore should be separated a little bit imo

---

This is an independent effort from #10597, but as it's still a Book Chapter Update, I thought it would be cool to include it here. I'm going to keep the reviewing process for this PR to rustbot's desires.

[Rendered](https://github.com/blyxyas/rust-clippy/blob/book-lint_config/book/src/lint_configuration.md)
[Current](https://github.com/rust-lang/rust-clippy/blob/master/book/src/lint_configuration.md)

changelog: Refresh styling from the "Lint Configuration Options" book chapter.
@xFrednet
Copy link
Member

xFrednet commented Jun 3, 2023

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:

  • Avoid unchecked indexing, this can cause ICEs. Prefer refutable slice patterns
  • For new lints, do they handle macros?
  • For lints checking method names, is the type also checked.
  • For applicability > Unspecified, is there a .fixed file and the //@run-rustfix comment
  • Does the lint follow rustc's lint naming conventions
  • Does the lint follow rustc's error messages style
  • Are new config values tested in cargo-ui tests
  • Does the .stderr file only contain relevant lint triggers

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

@blyxyas
Copy link
Member Author

blyxyas commented Jul 31, 2023

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 🐮

bors added a commit that referenced this issue Aug 16, 2023
Clippy Book Chapter Updates Reborn: Defining Lints

Revival of #9659, I've made a few changes (mainly ones from reviews from #9426, like removing mid-section storytelling) and some re-writes. The goal of the project would be to slowly re-write the Clippy book (only chapters that need it) to more up-to-date style.

## Notes

- Things like `git status` commands have changed, we need to make sure that they're correct.
- As this is a team-wide effort, I would please ask anyone and everyone to read it and give your opinion.
- To talk about the whole project, please use the tracking issue for the project #10597  (It also contains a timeline, discussions and more information)

---

changelog: Add a new "Defining lints" chapter to the book

r? `@flip1995`
bors added a commit that referenced this issue Aug 16, 2023
Clippy Book Chapter Updates Reborn: Method Checking

This PR adds a new chapter to the book: "Method Checking". Some major re-phrasing was done and some changes in the code comments (apart from grammar and minor changes).

## Notes

- Requires #10595 **and** #10622 to be merged before this, or else several links will be broken
- To talk about the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions and more information)

changelog: Add a new "Method Checking" chapter to the book.

r? `@flip1995`
@Centri3
Copy link
Member

Centri3 commented Aug 18, 2023

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.

bors added a commit that referenced this issue Sep 2, 2023
Clippy Book Chapter Updates Reborn: Writing tests

This PR adds  a new chapter to the book: "Writing tests". The changes have been mainly done from reviews from #9426 and some minor re-writes.

## Notes

- We still need to check that the `git status`es are correct, as `cargo dev new_lint` changed a lot since 2022.
- Requires #10598: Link to "Emitting Lints" where I flagged with `FIXME:`.
- To talk about the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions and more information)

changelog: Add a new "Writing tests" chapter to the book
r? `@flip1995`
bors added a commit that referenced this issue Sep 2, 2023
Clippy Book Chapter Updates Reborn: Emitting lints

The PR adds a new chapter to the book: "Emitting lints". This time it changed a lot from the old source file.

## Notes

- For discussion about  the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions, and more information)

changelog: Add a new "Emitting lints" chapter to the book

r? `@flip1995`
bors added a commit that referenced this issue Sep 2, 2023
Clippy Book Chapter Updates Reborn: Emitting lints

The PR adds a new chapter to the book: "Emitting lints". This time it changed a lot from the old source file.

## Notes

- For discussion about  the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions, and more information)

changelog: Add a new "Emitting lints" chapter to the book

r? `@flip1995`
bors added a commit that referenced this issue Sep 2, 2023
Clippy Book Chapter Updates Reborn: Trait Checking

This PR adds a new chapter to the book: "Trait Checking". No major changes from the source (just some typos, re-phrasing, the usual).

## Notes

- Does not require any other PR to be merged.
- To talk about the whole project, please use the tracking issue for the project #10597 (It also contains a timeline, discussions and more information)

changelog: Add a new "Trait Checking" chapter to the book
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation C-tracking-issue Category: Tracking Issue
Projects
None yet
Development

No branches or pull requests

4 participants