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

Allen Wittenauer edited comment on YETUS-488 at 5/17/17 1:26 AM:
-----------------------------------------------------------------

Just to follow up on this, I did more testing today, again using YARN-6363 and 
hadoop 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.


was (Author: aw):
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: branch-checkstyle-hadoop-tools.txt, 
> diff-checkstyle-hadoop-tools.txt, patch-checkstyle-hadoop-tools.txt, 
> 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)

Reply via email to