10

New lint [`min_ident_chars`] by Centri3 · Pull Request #10916 · rust-lang/rust-c...

 1 year ago
source link: https://github.com/rust-lang/rust-clippy/pull/10916
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Contributor

@Centri3 Centri3

commented

Jun 9, 2023

edited by xFrednet

Closes #10915

This also implements WithSearchPat for Ident, as I was going to rewrite this as a late lint to optionally ignore fields and/or generics but this was more complex than anticipated

changelog: New lint [min_ident_chars]
#10916

Collaborator

r? @Jarcho

(rustbot has picked a reviewer for you, use r? to override)

rustbot

added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label

Jun 9, 2023

Member

Hey @blyxyas do you want to review this PR? It sounds like a good and simple lint :)

Assuming it's fine for @Jarcho if we steal the review.

r? @xFrednet

Contributor

Yes, will take it :)

Centri3

changed the title New lint [single_letter_idents]

New lint [single_char_idents]

Jun 9, 2023

Contributor

@blyxyas blyxyas

left a comment

Some really really nitpicks. Everything else looks fine, Thanks heart

Contributor

@blyxyas blyxyas

left a comment

Everything seems pretty fine, thanks! Seems like a very useful lint heart

cc @xFrednet

xFrednet reacted with heart emoji

Member

You just pushed, while I was doing a review xD

It looks like that deleted a comment, let's see if I can find that one again

Contributor

Author

Sorry, had to make sure I ran collect-metadata on this one (spoiler alert: I did not.)

xFrednet reacted with laugh emoji

Member

@xFrednet xFrednet

left a comment

Hey, so far, I've only looked at the tests and config implementation. It looks like you're also still working on the branch. These are some nits I found rn. Let me know when you're ready for the next review :)

Member

Sorry, had to make sure I ran collect-metadata on this one (spoiler alert: I did not.)

No problem! It was not the first time this happened, just the first time that GH deleted a comment. It was a simple one to reconstruct though :D

Contributor

Author

Let me know when you're ready for the next review :)

Feel free to take a look whenever, the implementation is already pretty much done.

Member

I looks like the PR has gotten some conflict, without bors adding a comment. Could you resolve them?

Member

@xFrednet xFrednet

left a comment

This version looks quite good to me. I have three tiny nits, and then it should be ready :)

Contributor

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

Centri3

changed the title New lint [single_char_idents]

New lint [min_ident_chars]

Jun 12, 2023

Centri3

changed the title New lint [min_ident_chars]

New lint [min_ident_chars]

Jun 12, 2023

Member

Looks good to me, thank you!

@bors r=blyxyas,xFrednet

Contributor

pushpin Commit 29c1c6e has been approved by blyxyas,xFrednet

It is now in the queue for this repository.

Contributor

hourglass Testing commit 29c1c6e with merge a3b185b...

Contributor

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

Reviewers

xFrednet

xFrednet approved these changes

blyxyas

blyxyas approved these changes
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.

single_letter_ident

6 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK