Mailing-List: contact notifications-help@yetus.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@yetus.apache.org
Date: Wed, 15 Feb 2017 20:02:41 +0000 (UTC)
From: "Allen Wittenauer (JIRA)" <jira@apache.org>
To: notifications@yetus.apache.org
Message-ID: <JIRA.13043279.1487170360000.90802.1487188961830@Atlassian.JIRA>
In-Reply-To: <JIRA.13043279.1487170360000@Atlassian.JIRA>
References: <JIRA.13043279.1487170360000@Atlassian.JIRA> <JIRA.13043279.1487170360892@jira-lw-us.apache.org>
Subject: [jira] [Comment Edited] (YETUS-488) Checkstyle reports new error if
 the file still longer than expected
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Wed, 15 Feb 2017 20:02:46 -0000


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