Conversation
8805d36 to
0ae307f
Compare
0ae307f to
0c72e9d
Compare
|
☔ The latest upstream changes (presumably #13322) made this pull request unmergeable. Please resolve the merge conflicts. |
0c72e9d to
60e1995
Compare
bffedc4 to
1867997
Compare
GuillaumeGomez
left a comment
There was a problem hiding this comment.
Apart from the missing ui test annotations, looks good to me, nice work!
1867997 to
ee3d384
Compare
|
I've added the missing test annotations, thanks for the review! |
ee3d384 to
56ae4b8
Compare
|
@rustbot ready |
|
Looks all good to me, thanks! Let's just wait someone from the team to approve it too. :) |
56ae4b8 to
8fe840d
Compare
8fe840d to
538193b
Compare
y21
left a comment
There was a problem hiding this comment.
Looks pretty good to me already, most of the comments I initially had were mentioned by others and are now resolved :)
538193b to
85922db
Compare
7da77c0 to
d90eefa
Compare
|
@y21 Do you see something that needs to be changed? |
|
I'll try to take another look later today |
|
FCP1 thread: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/FCP.3A.20needless_as_bytes Footnotes
|
|
@y21 Any news? |
|
Sorry, have not had much time the last few days. There are some comments on the Zulip thread suggesting that this is something that some people do intentionally write to make it clear that it's the byte length (and this argument is also in the linked issue's drawbacks), so I'd want to see a poll on the Zulip for the category. I'll try to do it later today. I'm personally in favor of this lint being warn-by-default, but we shouldn't have warn-by-default lints that people find hurts readability. |
|
@y21 I think it's also worth weighing the possibility of people simply not knowing That said, as long as the lint exists in any capacity, I would be happy with the results. |
y21
left a comment
There was a problem hiding this comment.
It's been about a week since I've posted the poll on zulip and the majority agrees with the current category (complexity, warn-by-default), so I'd say we're good to go.
Can you update the #[clippy::version] attribute? Also, you'll probably have to rebase and run cargo dev update_lints since this is going to be the 750th lint
clippy_lints/src/methods/mod.rs
Outdated
| /// let len = "some string".len(); | ||
| /// let b = "some string".is_empty(); | ||
| /// ``` | ||
| #[clippy::version = "1.83.0"] |
There was a problem hiding this comment.
| #[clippy::version = "1.83.0"] | |
| #[clippy::version = "1.84.0"] |
d90eefa to
f152bcb
Compare
Updated |
|
Thanks (also to @GuillaumeGomez and @LikeLakers2 for helping with reviewing)! @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
I ran Clippy on some projects after upgrading to 1.84, which found a [needless use of `as_bytes()`](#13437). It made me notice that the code was also using `bytes()` needlessly in other places. This PR improves on the `as_bytes()` lint to also lint `bytes()`. ---- changelog: none
changelog: [
needless_as_bytes]: new lintFix #13434