Maven 2.x Checkstyle Plugin

checsktyle:check seems to fail on warning too

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.1
  • Labels:
    None
  • Number of attachments :
    1

Description

failOnViolation=true is supposed to fail only on errors but it seems to be failing on warnings too. See attached checkstyle result file which leads to

[...]
[INFO] [checkstyle:check {execution: default}]
[INFO] ------------------------------------------------------------------------
[ERROR] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] You have 194 checkstyle violations.
[INFO] ------------------------------------------------------------------------

I think the problem is that the code checks for an <error> tag whereas it should check for the "severity" attribute instead.

Issue Links

is related to

New Feature - A new feature of the product, which has yet to be developed. MCHECKSTYLE-45 It should be possible to configure checkstyle:check to fail on "warnings".

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Brett Porter added a comment -

since this was in a release version, I'm not going to change the pending RC which was already voted on.

Is it unreasonable to fail on warnings? Would introducing a parameter that let them choose, defaulting to the current behaviour be acceptable?

Show
Brett Porter added a comment - since this was in a release version, I'm not going to change the pending RC which was already voted on. Is it unreasonable to fail on warnings? Would introducing a parameter that let them choose, defaulting to the current behaviour be acceptable?
Hide
Permalink
Vincent Massol added a comment -

Is it unreasonable to fail on warnings?

Yes it's unreasonable as this is not the behavior of checkstyle itself. Also, Maven 1.x checkstyle plugin does not fail on warnings by default.

Would introducing a parameter that let them choose, defaulting to the current behaviour be acceptable?

Yes this is the right thing to do. I think the default should be not to fail on warnings though as this is what checkstyle itself is configured to do and this is also the default behavior of the maven 1.x plugin. I'll leave that up to you to decide.

Thanks

Show
Vincent Massol added a comment - Is it unreasonable to fail on warnings? Yes it's unreasonable as this is not the behavior of checkstyle itself. Also, Maven 1.x checkstyle plugin does not fail on warnings by default. Would introducing a parameter that let them choose, defaulting to the current behaviour be acceptable? Yes this is the right thing to do. I think the default should be not to fail on warnings though as this is what checkstyle itself is configured to do and this is also the default behavior of the maven 1.x plugin. I'll leave that up to you to decide. Thanks
Hide
Permalink
Brett Porter added a comment -

ok, that certainly could be considered a bug in its current state, and its not going to regress on someone (the worst is it will fail now and not fail later, but they are unlikely to be using it if it is failing). I'll include it.

Got a patch?

Show
Brett Porter added a comment - ok, that certainly could be considered a bug in its current state, and its not going to regress on someone (the worst is it will fail now and not fail later, but they are unlikely to be using it if it is failing). I'll include it. Got a patch?

People

  • Assignee:
    Brett Porter
    Reporter:
    Vincent Massol
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: