Skip to content
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

move upper_case_acronyms back to style, but make the default behaviour less aggressive by default (can be unleashed via config option) #6788

Merged
merged 3 commits into from
Feb 25, 2021

Conversation

matthiaskrgr
Copy link
Member

Previous discussion in the bi-weekly clippy meeting for reference: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Meeting.202021-02-23/near/227458019

Move the upper_case_acronyms lint back to the style group.
Only warn on fully-capitalized names by default.
Add add a clippy-config option upper-case-acronyms-aggressive: true/false to enabled more aggressive linting on
all substrings that could be capitalized acronyms.


changelog: reenable upper_case_acronyms by default but make the more aggressive linting opt-in via config option

@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 24, 2021
Moves the lint back from pedantic to style group.
The lint default now only warns on names that are completely capitalized, like "WORD"
and only if the name is longer than 2 chars (so that names where each of the letter represents a word are still distinguishable).
For example: FP (false positive) would still be "valid" and not warned about (but EOF would warn).

A "upper_case_acronyms_aggressive: true/false" config option was added that restores the original lint behaviour to warn
on any kind of camel case name that had more than one capital letter following another capital letter.
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.

Only a NIT. r=me otherwise.

clippy_lints/src/utils/conf.rs Outdated Show resolved Hide resolved
fix sentence / address review comments
@matthiaskrgr
Copy link
Member Author

@bors r=flip1995

@bors
Copy link
Contributor

bors commented Feb 25, 2021

📌 Commit 2a6b061 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Feb 25, 2021

⌛ Testing commit 2a6b061 with merge ef41f2b...

@bors
Copy link
Contributor

bors commented Feb 25, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing ef41f2b to master...

@bors bors merged commit ef41f2b into rust-lang:master Feb 25, 2021
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.

5 participants