-
Notifications
You must be signed in to change notification settings - Fork 134
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
Remove DefaultLocale in favor of StringCaseLocaleUsage #2589
Conversation
// StringCaseLocaleUsage duplicates our existing DefaultLocale check which is already | ||
// enforced in some places. | ||
"StringCaseLocaleUsage", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This effectively changes this check from SUGGESTION
to WARNING
, but we've had an excavator for this for a while now, so this should be acceptable.
// StringCaseLocaleUsage duplicates our existing DefaultLocale check which is already | ||
// enforced in some places. | ||
"StringCaseLocaleUsage", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see 5 references to DefaultLocale
in our repos:
- 3 are increasing the severity to
WARNING
orERROR
, so this change is fine for those repos - 2 are disabling this check, so those 2 repos will need to replace
DefaultLocale
withStringCaseLocaleUsage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this change is fine for those repos
I believe compilation will fail because no check exists with name DefaultLocale
after this PR merges
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are the PRs for 2 of those 3. I'll also put up a PR for the one internal repo:
- Remove DefaultLocale override atlasdb#6605 (AtlasDB's baseline upgrade is blocked on other manual changes anyways)
- Remove DefaultLocale override dialogue#1946
@carterkozak this should be good to merge now |
@carterkozak any other concerns here? |
Closing in favor of #2839 |
This check is now provided by Error Prone, so we no longer need our custom
DefaultLocale
check.