search

spotbugs / spotbugs-maven-plugin

Maven Mojo Plug-In to generate reports based on the SpotBugs Analyzer
https://spotbugs.github.io/spotbugs-maven-plugin/
Apache License 2.0
69 stars 51 forks source link

CheckMojo: introduce failThreshold #222

Closed famod closed 3 years ago

famod commented 3 years ago

Resolves #219

Notes:

famod commented 3 years ago

I just realized that your Travis build uses the latest commits anyway so I rebased my branch. This will (of course) fail all the builds for this PR until the TestNG problem is fixed.

famod commented 3 years ago

@hazendaz WDYT?

hazendaz commented 3 years ago

travis ci is dicey these days. I tried to rebuild this yesterday but no luck. Issue was not groovy but rather the invoker pulling testng. I don't see us using testng but it's having issues getting it. I tried running it again tonight. I'd like to see travis successful. I also need to wrap my head around the changes a bit more. A quick look seems ok but just want to review some of the naming changes and make sure it looks appropriate for current usage. So I'm on board with it just need to get travis working and look at it in a little more depth. I'll try to get to it in next couple of days. If travis continues to fail, can you take a look at what might be causing that? I'm pretty sure when I upgraded groovy it was fine on that pull request. I usually skip merging if travis is having issues unless its related to pulling a lib it cannot find. That could have been the case on the groovy update. I don't recall off hand.

famod commented 3 years ago

@hazendaz

Well, the Groovy upgrade PR ran into the same problem: https://github.com/spotbugs/spotbugs-maven-plugin/pull/221 I can reproduce it locally, so this is not a Travis problem, AFAICS.

hazendaz commented 3 years ago

@famod Thanks. I'll take a look tonight.

famod commented 3 years ago

@hazendaz friendly reminder! đŸ™‚

hazendaz commented 3 years ago

I need to ping testng folks I think to get them to release their binary. Their github states it was released but obviously wans't in central last I checked. I kicked off another build just now. If it fails again, I'll reach out to that team.

famod commented 3 years ago

So it failed again.

IMHO, the groovy update should be rolled back for now.

hazendaz commented 3 years ago

@famod Rebuilding now. Issue fixed. We don't use testng but the groovy plugin uses groovy all which we override to latest. Per groovy, they messed up by using testng on version not released to central, testng has since removed notice of testng release. Since we don't use testng, I followed groovy's work around to exclude it from the groovy plugin and builds are back to normal. As soon as this goes green, I'll review once again and then get it merged.

hazendaz commented 3 years ago

@famod Thank you! Merging now.

famod commented 3 years ago

@hazendaz Awesome, thanks! Any plans on cutting a new release?

hazendaz commented 3 years ago

@famod I'm waiting on a pull request to be corrected so the test for htm vs xml works. I'd like to include that. So hopefully soon. If I don't hear anything back this coming week on the other pull request, I'll likely just move fowards without it.

famod commented 3 years ago

Thanks for your feedback!

famod commented 3 years ago

@hazendaz Any news regarding a new release? Thanks!

hazendaz commented 3 years ago

@famod I'll release this tonight.

hazendaz commented 3 years ago

@famod Released, it will be available in central in next 2 hours or so. You should be able to otherwise pull it probably in about 15 minutes or so.

hazendaz commented 3 years ago

Version 4.0.4 to match line of spotbugs.

famod commented 3 years ago

@hazendaz Great, thanks!