-
Notifications
You must be signed in to change notification settings - Fork 6
Updated README.md #34
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
Conversation
romani
left a comment
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.
Thoughts
README.md
Outdated
| _No checks analyzed yet_ | ||
| | Status | Check | Recipe | Coverage Notes | | ||
| |--------|--------------------------------------------------------------------------------------------------------------------------------------|--------|-----------------------------------------| | ||
| | 🟢 | [`AbbreviationAsWordInName`](https://checkstyle.sourceforge.io/checks/naming/abbreviationaswordinname.html#AbbreviationAsWordInName) | `TBD` | | |
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 don't know how it can be auto fix?
We don't know what user meant
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.
can be considered as partial coverage !!
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.
https://docs.openrewrite.org/recipes/staticanalysis/renamelocalvariablestocamelcase
We should be very accurate.
They look like do renaming from upper case snake style to camel only. Where _ is separator of words. Only in this rare edge case it is reliable.
README.md
Outdated
| | Status | Check | Recipe | Coverage Notes | | ||
| |--------|--------------------------------------------------------------------------------------------------------------------------------------|--------|-----------------------------------------| | ||
| | 🟢 | [`AbbreviationAsWordInName`](https://checkstyle.sourceforge.io/checks/naming/abbreviationaswordinname.html#AbbreviationAsWordInName) | `TBD` | | | ||
| | 🟢 | [`AbstractClassName`](https://checkstyle.sourceforge.io/checks/naming/abstractclassname.html#AbstractClassName) | `TBD` | | |
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 don't know how it can be auto fix?
We don't know what user meant .
It has two options to fix .
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.
The check uses name pattern to check and other two properties to control when to check. I think we can try to cover it by having a recipe that accepts same pattern and does renaming according to the pattern. It won't be fully covered, but we can try to cover the check partially at least.
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.
@Anmol202005 let's change it to "Partial Coverag" for now, we can always set "not covered" if nothing we can do
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.
done.
d8d5888 to
c202db4
Compare
|
@romani updated both as not fixable :) |
|
@Anmol202005 let's put all such commits under this issue #2 |
@rdiachenko done. |
|
@rdiachenko Done. |
rdiachenko
left a comment
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.
lgtm
Part of #2:
Updated README.md added: