Skip to content

Add separate plugin parameter to filter dependencies #400

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Marcono1234
Copy link
Contributor

Enclosing `if` statement is not needed because in case there are no
filter names `createFilterString` will create an empty string.
To ensure that no API newer than the target version is used by accident,
when building with a newer JDK.
Comment on lines -627 to +634
if (inFilter != null) {
dependencyInjarFilterList.add(inFilter);
if (inDependenciesFilter != null) {
dependencyInjarFilterList.add(inDependenciesFilter);
Copy link
Contributor Author

@Marcono1234 Marcono1234 Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another alternative to adding a new parameter could be to reuse the existing inLibsFilter for this case here as well. That might be a bit more appropriate than the previous inFilter.

But maybe adding a new parameter (as currently done in this pull request) is still the cleanest approach.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backwards compatibility this could use inFilter if inDependenciesFilter is null but inFilter non-null. But maybe it is better to not do too much complexity in name of compatibility. This is an easy change for users if they use the filters.

<source>1.8</source>
<target>1.8</target>
<release>8</release>
Copy link
Contributor Author

@Marcono1234 Marcono1234 Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: This will require JDK >= 9 for building because it uses javac --release, but I guess it is safe to assume that most users will have that.

(This is not technically needed for the other changes in this pull request.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requiring JDK 9 is fine. We could even change this to 11. I have no intention to maintain this for older versions.

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

Successfully merging this pull request may close these issues.

2 participants