Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support a SpotBugs exclude filter file for Maven multi-module projects #631

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

basil
Copy link
Member

@basil basil commented Nov 3, 2022

Like jenkinsci/pom#337 but for plugin-pom. I tested this logic as part of the other PR.

@basil basil added the bug label Nov 3, 2022
@basil basil merged commit 7a900a5 into jenkinsci:master Nov 3, 2022
@basil basil deleted the excludeFilterFile branch November 3, 2022 15:14
@jtnord
Copy link
Member

jtnord commented Nov 9, 2022

What was the rationale here, the filter already worked for multi-module projects - you just put the the exclude file alongside your modules projects source?

Additionally the directory used will seem to be incorrect if you have an aggregator project to build multiple other projects (that are not in the same github repo - e,g. using submodules, or extra clones).

Additionally why would someone not want to have their exclude file in the location with their module (which it corresponds to ) as opposed to a higher level?

e.g.
https://github.com/cloudbees/cloudbees-cyberark-credentials-plugin/blob/master/cyberark-aim-stub/src/spotbugs/excludesFilter.xml shows this was working before and thus no change should have been required.

@amuniz
Copy link
Member

amuniz commented Nov 9, 2022

Agree. This inconvenient and possibly breaking some of our builds.

@basil
Copy link
Member Author

basil commented Nov 9, 2022

Do you have a link to a consumer in the jenkinsci GitHub organization that is broken by this change?

@basil
Copy link
Member Author

basil commented Nov 19, 2022

I did a corpus study of the repositories in jenkinsci with a multi-module build and a SpotBugs excludes file. It turned out that in most cases they were not really benefiting from a centralized exclusion file, and most of the exclusions applied only to one module in the multi-module build. In light of this and the downsides mentioned above, I will revert this change.

@basil
Copy link
Member Author

basil commented Nov 19, 2022

Thanks for bringing this to my attention James. I released POM 1.92 and plugin POM 4.51 with project.basedir and filed the following PRs to adapt consumers:

I think the result turned out nicely with the exclusion logic living closer to the code being excluded, resulting in improved readability.

@amuniz
Copy link
Member

amuniz commented Nov 21, 2022

Thanks Basil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants