[
https://issues.apache.org/jira/browse/YETUS-488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15868478#comment-15868478
]
Allen Wittenauer edited comment on YETUS-488 at 2/15/17 8:02 PM:
-----------------------------------------------------------------
bq. I am even hesitant about the first one, because the change I introducing
with it might have unwanted consequences.
A lot of the difference code prior to calcdiffs being implemented (a while back
now!) stripped this data down to just the error and used this FIFO method. The
result was a mess because you'd end up with:
base:
error1
error1
error2
patch:
remove error1
introduce new error1
error1
remove error2
new error2
e.g., the errors would be for new lines of code that the patch introduced in
the process of removing the old, problematic code. But since the ordering was
the same, they weren't flagged. The line number and column checks were
specifically added in to make sure new errors showed up as new errors.
was (Author: aw):
bq. I am even hesitant about the first one, because the change I introducing
with it might have unwanted consequences.
A lot of the difference code prior to calcdiffs being implemented (a while back
now!) stripped this data down to jus the error and used this FIFO method. The
result was a mess because you'd end up with:
old code:
error1
error1
error2
new2:
remove error1
introduce new error1
error1
remove error2
new error2
e.g., the errors would be for new lines of code that the patch introduced in
the process of removing the old, problematic code. But since the ordering was
the same, they weren't flagged. The line number and column checks were
specifically added in to make sure new errors showed up as new errors.
> Checkstyle reports new error if the file still longer than expected
> -------------------------------------------------------------------
>
> Key: YETUS-488
> URL: https://issues.apache.org/jira/browse/YETUS-488
> Project: Yetus
> Issue Type: Improvement
> Components: Test Patch
> Affects Versions: 0.4.0
> Reporter: Peter Vary
> Assignee: Peter Vary
> Priority: Minor
> Attachments: YETUS-488.00.patch
>
>
> Given a java source file which is originally longer than the checkstyle
> defined value then we get the following warning every time the length changes
> and we run the checkstyle plugin:
> {code}
> ./beeline/src/java/org/apache/hive/beeline/BeeLine.java:1: warning: File
> length is 2,373 lines (max allowed is 2,000).
> {code}
> The problem is, that the checkstyle output is changed:
> {code:title=From}
> ./beeline/src/java/org/apache/hive/beeline/BeeLine.java:1: warning: File
> length is 2,373 lines (max allowed is 2,000).
> {code}
> {code:title=To}
> ./beeline/src/java/org/apache/hive/beeline/BeeLine.java:1: warning: File
> length is 2,365 lines (max allowed is 2,000).
> {code}
> Since the {{checkstyle_calcdiffs}} does not find the new line in the original
> output, it marks it as a new error, and gives -1 to the result of the
> checkstyle plugin.
> This is true for every error message where there is a number in the text. Not
> exhaustive examples are below:
> - Line length
> - Row length
> - Function length
> - Number of attributes
> - Indentation
> I think we should be reluctant to give -1 without a reason so it would be
> good to remove these errors.
> I have yet to find the ideal solution for the issue:
> - An easy patch could be to remove the numbers from files before the diff
> (note: it does not effect the final diff-checkstyle-<MODULE>.txt output
> messages). This would mean that the warning will give -1 only the first time
> when the error text is occurred, and will not give -1 if the numbers are
> changed, like
> -- File become shorter, but still longer than expected
> -- File become even longer
> -- Indentation changed but still problematic
> - A more sophisticated patch could do this only for specific errors, or even
> check the value and give error when the line length grow.
> To be honest, I think the second solution would complicate the code too much,
> and make it dependent on the specific checkstyle messages, so I do not like
> it.
> I am even hesitant about the first one, because the change I introducing with
> it might have unwanted consequences.
> Do anyone has better ideas? I would be happy to implement them, if someone
> points me to the right direction :D
> Thanks,
> Peter
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
