[jira] [Commented] (YETUS-488) Checkstyle reports new error if the file still longer than expected

["Allen Wittenauer (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YETUS-488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16013354#comment-16013354 ] 

Allen Wittenauer commented on YETUS-488:
----------------------------------------

Just to follow up on this, I did more testing today, again using YARN-6363 and git trunk at hex f356f0f4cf.

From what I can tell, this latest version does appear to be more accurate but it's still dropping info.  I think I'd like to see if we can fix the last little bump before we commit though.  I'll attach the (complete) txt files that appear to be throwing it off to help in debugging.

> 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, YETUS-488.01.patch, YETUS-488.02.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)