kordamp / pomchecker

🦉 Maven POM syntax checker
Apache License 2.0
65 stars 10 forks source link

Maven plugin: support for a "warning only" mode #9

Closed Stephan202 closed 1 year ago

Stephan202 commented 1 year ago

First off: thanks for providing this nice project!

Maven plugins that perform some kind of validation often provide a flag to toggle whether violations should fail the build, or are merely logged. I was wondering whether you have considered adding a similar feature for this project's Maven plugin? For examples see this Maven profile, where the relevant properties have names such as failOnError, failOnViolation and failOnWarning.

Context for this request: the requested feature would simplify PicnicSupermarket/error-prone-support#491, and allow the plugin to participate in our build's warn-but-don't-fail mode.

aalmiray commented 1 year ago

Thanks 😅

I think adding a failOnError setting would be the way to go.

aalmiray commented 1 year ago

@Stephan202 FWIW I believe pomchecker does not fail build when a warning is emitted, only when there are errors. Do you also want to suppress warnings?

Stephan202 commented 1 year ago

Do you also want to suppress warnings?

Good question. I'm tending to "yes", but I'm not sure that blanket suppression is most useful. For example, if we decide to fix this warning in our code, then afterwards I'd want to guard against new violations by raising its severity to an error. But I realize that this significantly expands the scope of this feature request from "add a failOnError flag" to "allow each check to be assigned a severity of OFF, WARN or ERROR, and fail if there are any ERRORs." :grimacing:

aalmiray commented 1 year ago

I see. I'm not so sure if every check should have it's own status as the whole point of this tool is to raise awareness of missing XML elements. There are however two types of elements:

  1. required. Should trigger ERROR when missing. Maven Central rules will trigger when a pom is uploaded.
  2. explicitly missing but inherited. They trigger WARN because their value is not explicitly set. These are perhaps the ones that could have a severity assigned as their inferred value should not cause issues when Maven Central checks them.

Thus, I can add a failOnError flag to skip failing the build when needed, as well as check states for elements of type 2.

WDYT?

Stephan202 commented 1 year ago

@aalmiray yes, that makes sense to me :+1:

aalmiray commented 1 year ago

@Stephan202 I've added failOnError and failOnWarning properties. Do let me know if additional fine grained control for <url>, <repositories>, and <pluginRepositories> is needed.

Stephan202 commented 1 year ago

@aalmiray I just tested it by modifying the PR mentioned in the issue description: it works very nicely! Thanks for implementing this feature! :rocket:

aalmiray commented 1 year ago

Alright. I'll go ahead and release 1.8.0 this weekend.

aalmiray commented 1 year ago

Released in v1.8.0 -> https://github.com/kordamp/pomchecker/releases/tag/v1.8.0