yetus-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Allen Wittenauer (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (YETUS-488) Checkstyle reports new error if the file still longer than expected
Date Wed, 15 Feb 2017 20:02:41 GMT

    [ 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)

Mime
View raw message