Skip to content

Changelog for Clippy 1.87 🦡 #14748

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

Merged
merged 1 commit into from
May 12, 2025
Merged

Conversation

alex-semenyuk
Copy link
Member

Violets are red,
Roses are blue,
It’s wrong on purpose —
We don’t break tradition.


This is cat-penguin of this release nominated by @xFrednet some time ago:

image

Cats for the next release can be traditionally nominated in the comments :D
Please be more active and cat-minded 😻


changelog: none

r? flip1995

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 7, 2025
@alex-semenyuk alex-semenyuk marked this pull request as draft May 7, 2025 14:55
@alex-semenyuk alex-semenyuk force-pushed the changelog_1_87 branch 4 times, most recently from 18a78e1 to df16438 Compare May 7, 2025 15:13
@alex-semenyuk alex-semenyuk marked this pull request as ready for review May 7, 2025 15:42
Copy link
Member

@y21 y21 left a comment

Choose a reason for hiding this comment

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

Thanks for helping with the changelog! Some of the lint categories are incorrect (I assume you took them from the description and they were changed later on in the review process)

@y21
Copy link
Member

y21 commented May 7, 2025

The #[clippy::version] attributes will need to be updated as well for the newly added lints (e.g. owned_cow currently has #[clippy::version = "1.85.0"] when it should be 1.87.0), though I suppose that's not a blocker for this changelog. Last time, this was also done separately (#14540)

@xFrednet
Copy link
Member

xFrednet commented May 8, 2025

This made my day, and it's not even 10 am yet! :D

And you got all the little details with the poem, emoji and picture right. I love it, 10/10


The #[clippy::version] attributes will need to be updated as well for the newly added lints (e.g. owned_cow currently has #[clippy::version = "1.85.0"] when it should be 1.87.0), though I suppose that's not a blocker for this changelog. Last time, this was also done separately (#14540)

Good catch @y21. You don't need to split the changes into two commits. I liked it that way, but there is no other reason to do this.

In this regard, I can also share how I usually wrote them (In case this helps):

  1. Get the two commits and generate the changes.txt file
  2. I often wrote the changelog in a new changes.md file since VS code was taking more resources when editing our long CHANGELOG.md file
    • If you want to, you can filter out changes, which seem less relevant to everyone. People have the list of all merged PRs. This mainly saves time when writing it and distils the changes to the most relevant ones. But this is purely a style question, so find your own way.
  3. For each new lint, do a whole text search for pub <lint-name> to correct the #[clippy::version] attribute and copy the lint group into the changelog.
  4. Spellcheck changes.md
  5. Copy changes.md into CHANGELOG.md
  6. Commit, etc. This is a list of all emojis I’ve previously used in case you want to avoid duplicates. I simply tracked these in my Zulip chat: 🪄, 🏫, 🐈, 🎄, 🎃, 🖊️, 🏖️, 👑, 🔨, 🌸, 🐶, 🐞, 🎅, 🚥, 🍎, 🦖, 🐙, 🦀 🎓 🌞 🔰 🤖 🧨 🦜
      • 🐧 and 🦡 from this and the previous Changelog

I want to finish this again off with saying thank you! This is so cute and I love to see that this is carried forward! Because of this ping I also saw the previous changelog PR which is equally amazing. This makes the community special! ❤️

@xFrednet
Copy link
Member

xFrednet commented May 8, 2025

20250418_232956~2.jpg

And I want to nominate June as the cat of the next release. Look at that cute smile!

(Please ping me when June made it as the cat of the release. I want to send it to her owner :D)

@alex-semenyuk alex-semenyuk force-pushed the changelog_1_87 branch 2 times, most recently from 8b9c9e1 to e9fc233 Compare May 8, 2025 10:31
@alex-semenyuk alex-semenyuk requested a review from y21 May 8, 2025 10:42
@flip1995
Copy link
Member

flip1995 commented May 8, 2025

3. For each new lint, do a whole text search for pub <lint-name> to correct the #[clippy::version] attribute and copy the lint group into the changelog.

Pass -B1 to grep/rg to quickly scan for the #[clippy::version] in the terminal output (should you do it there)

Thanks for summarizing how to most efficiently update the versions! @alex-semenyuk could I ask you to also write this in our documentation?

* [`manual_flatten`] [#14086](https://github.com/rust-lang/rust-clippy/pull/14086)
* [`lines_filter_map_ok`] [#14130](https://github.com/rust-lang/rust-clippy/pull/14130)

### False Positive Fixes
Copy link
Member

Choose a reason for hiding this comment

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

After sleeping over this: I would like to keep the "new" style of only mentioning the most important changes, rather than all. This list of changes is really overwhelming IMO.

Sorry for not mentioning this sooner and you adding all of them :/ I know how much work this is...

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean "False Positive Fixes" section?

Copy link
Member

Choose a reason for hiding this comment

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

No, generally. See the previous changelogs, which are quite short. This is because the changelog author made judgements about what entries are important enough to list.

For example, I don't think that

[range_zip_with_len] now has an autofix

will be interesting to most people. On the other hand, changes to configuration values like the first 2 and the last point of ## Enhancements are worth to include in the changelog.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps something like this

### Enhancements

* Add `allow_expect_in_consts` and `allow_unwrap_in_consts` configuration options to [`unwrap_used`], [`expect_used`]
  [#14200](https://github.com/rust-lang/rust-clippy/pull/14200)
* Add `check-incompatible-msrv-in-tests` configuration option to [`incompatible_msrv`]
  [#14279](https://github.com/rust-lang/rust-clippy/pull/14279)
* [`len_zero`] now also triggers if deref target implements `is_empty()`
  [#13871](https://github.com/rust-lang/rust-clippy/pull/13871)
* [`ptr_eq`] now handles more cases, including `!=` in addition to `==`
  [#14339](https://github.com/rust-lang/rust-clippy/pull/14339)
* [`struct_field_names`] now also checks private fields of public structs
  [#14076](https://github.com/rust-lang/rust-clippy/pull/14076)
* [`needless_pass_by_value`] suggests using a reference on the innermost `Option` content
  [#14392](https://github.com/rust-lang/rust-clippy/pull/14392)
* [`obfuscated_if_else`] now supports `then().unwrap_or_else()` and `then_some().unwrap_or_else()`
  [#14165](https://github.com/rust-lang/rust-clippy/pull/14165)
* Format macros: all format-handling lints now validate `todo!` and `unimplemented!` macros
  [#14266](https://github.com/rust-lang/rust-clippy/pull/14266)
* [`disallowed_methods`] now supports replacements
  [#13669](https://github.com/rust-lang/rust-clippy/pull/13669)
* Added MSRV checks for several lints:
  * [`question_mark`] [#14436](https://github.com/rust-lang/rust-clippy/pull/14436)
  * [`repeat_vec_with_capacity`] [#14126](https://github.com/rust-lang/rust-clippy/pull/14126)
  * [`manual_flatten`] [#14086](https://github.com/rust-lang/rust-clippy/pull/14086)
  * [`lines_filter_map_ok`] [#14130](https://github.com/rust-lang/rust-clippy/pull/14130)

As for the rest sections they look quite short, except false positive, but not sure what can be removed from them, seems they're equal

Copy link
Member

Choose a reason for hiding this comment

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

This selection looks good!

I would say for the FP section, I would remove everything that doesn't give enough information on its own. For example [map_entry] Fix false positive ...: What FP? What was fixed? People won't click on a PR anyway and investigate what exactly was fixed. So those entries could be removed.

So think "Would I understand this change, if I was not working on Clippy, just from this one line?" And if the answer is "No", then it should probably be removed.

Copy link
Member

Choose a reason for hiding this comment

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

Oh and I think that the ### Suggestion Fixes/Improvements section can be completely removed (also from the docs). I don't think that anyone is interested in that. As in: "Oh this lint suggestion was fixed, so let me re-run cargo clippy --fix again". Probably doesn't happen, as they most likely already fixed the things by hand.

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.

Awesome thanks!

I personally might have slimmed it down even more, but that's just personal preference (of the changelog author). The current selection is definitely good! If you think those are important, there are others out there that will also find them important, so lets keep it as is.

Thanks again for writing it and addressing my (maybe annoying) review comments! ❤️

@flip1995 flip1995 added this pull request to the merge queue May 12, 2025
Merged via the queue into rust-lang:master with commit 2ce5451 May 12, 2025
11 checks passed
github-merge-queue bot pushed a commit that referenced this pull request May 12, 2025
Update "Changelog update walkthrough" section based on
#14748 discussion

changelog: none
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.

6 participants