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

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

OK, digging into this further, I'm wrong. I was misreading my sdiff. Hooray!

Unless anyone has anything to add, I'm leaning towards committing.  We can get 
some real world experience.  If it breaks, we'll back it out and try again.



> 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