-
Notifications
You must be signed in to change notification settings - Fork 2
Issue #53: updated test report and upperEll to use 1 based index #54
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
Issue for false-negative: #55 |
@@ -5,11 +5,11 @@ public class InputComplexLongLiterals { | |||
private long multipleUnderscores = 1_234_567_890l; | |||
|
|||
private long maxLong = 9223372036854775807l; | |||
private long minLong = -9223372036854775808l; | |||
private long minLong = -9223372036854775808l; //false negative |
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 understand this false negative
comment, please clarify what you 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.
these literals are not fixed by the recipe, due to improper column calculation.
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.
please confirm this will be fixed by #56
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.
yes, it will do.
@@ -1,109 +1,109 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<checkstyle version="10.12.3"> | |||
<file name="org/checkstyle/autofix/recipe/upperell/stringandcomments/InputStringAndComments.java"> | |||
<error line="11" column="30" severity="error" | |||
<error line="11" column="31" severity="error" |
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.
Note: We need to finish with integration test pipeline asap to get rid of these manual report files and fully rely on actual checkstyle execution and report
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.
yep, this is the way forward.
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
Fixes: #53
updated recipe to use 1 based index.