Add target* params for conflict targeting

[clairemcginty]

Adds targetDestinationPackages and targetSourcePackages parameters, which operate as the inverse of ignoreDestinationPackages and ignoreSourcePackages -- they allow the user to specify a list of packages that they only want to see conflicts from.

There are some breaking changes to IgnoredPackage class because I repurposed it to use for both types of filters - am hoping that the low adoption of missinglink makes these changes not such a big issue.

[pettermahlen]

Maybe 'inspect', 'consider', 'scan' instead of 'allow'?

On Thu, 1 Dec 2022 at 20:40, Matt Brown @.***> wrote:

@.**** commented on this pull request.

In README.md https://github.com/spotify/missinglink/pull/266#discussion_r1037491507:

or <ignoreDestinationPackage> element.

+### Allow only conflicts from certain packages

+

+Conversely, conflicts can be allowed based on the package name of the class that has the

+conflict. There are separate configuration options for allowing conflicts on

+the "source" side of the conflict and the "destination" side of the conflict.

+

+Packages on the source side can be allowe with <allowSourcePackages> and

⬇️ Suggested change

-Packages on the source side can be allowe with <allowSourcePackages> and

+Packages on the source side can be allowed with <allowSourcePackages> and

---

In README.md https://github.com/spotify/missinglink/pull/266#discussion_r1037494174:

or <ignoreDestinationPackage> element.

+### Allow only conflicts from certain packages

Maybe this is a bit meta, but if I am understanding right it is not really that you want to "allow" conflicts in these packages, but rather this would be used to make missinglink report an error only when the conflict happens in these packages, is that right?

I wonder if using "allow" in the description and the property name would be confusing then, but at the same time I'm not sure what a better term would be - maybe something like "failOnConflictInPackages" or "target packages", what do you think?

Or maybe something along the lines of "packages to look for conflicts in"

• its hard to think of a single term that describes this (unless I am just having a brain freeze)

In maven-plugin/src/main/java/com/spotify/missinglink/maven/PackageFilter.java https://github.com/spotify/missinglink/pull/266#discussion_r1037497201:

}

• public void setIgnoreSubpackages(boolean ignoreSubpackages) {

• this.ignoreSubpackages = ignoreSubpackages;

• public void setFilterSubpackages(boolean filterSubpackages) {

I wonder how hard it would be to support the old XML property name as well

• if I remember right, I think Maven is basically doing some sort of XML-to-java-bean translation so that true results in calling this setter method - so perhaps it would be possible to support the old XML property name by just adding an extra setter matching the old name too? i.e.

public void setIgnoreSubpackages(boolean ignoreSubpackages) {

this.filterSubpackages = ignoreSubpackages;

}

I think the actual class renaming is fine, that is probably invisible to Maven more or less

— Reply to this email directly, view it on GitHub https://github.com/spotify/missinglink/pull/266#pullrequestreview-1201561055, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACWLF4U3DBG4HN7374VRBDWLD5L3ANCNFSM6AAAAAASRA56BA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[clairemcginty]

@pettermahlen How about targetPackages ? That feels intuitive to me

[codecov[bot]]

Codecov Report

Merging #266 (6ec6a21) into master (8cca66e) will increase coverage by 0.10%. The diff coverage is 82.50%.

@@             Coverage Diff              @@
##             master     #266      +/-   ##
============================================
+ Coverage     85.10%   85.20%   +0.10%     
- Complexity      227      237      +10     
============================================
  Files            23       23              
  Lines           846      872      +26     
  Branches        100      104       +4     
============================================
+ Hits            720      743      +23     
- Misses           90       92       +2     
- Partials         36       37       +1     

Impacted Files	Coverage Δ
.../java/com/spotify/missinglink/maven/CheckMojo.java	90.07% <80.00%> (-0.28%)	:arrow_down:
...a/com/spotify/missinglink/maven/PackageFilter.java	70.37% <90.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mattnworb]

that codecov/patch check seems a little silly