Skip to content

Implementing #684: allow providing ruleSet in POM#686

Merged
slawekjaranowski merged 1 commit intomojohaus:masterfrom
andrzejj0:issue-684-property-based-rules-ruleset
Sep 13, 2022
Merged

Implementing #684: allow providing ruleSet in POM#686
slawekjaranowski merged 1 commit intomojohaus:masterfrom
andrzejj0:issue-684-property-based-rules-ruleset

Conversation

@andrzejj0
Copy link
Copy Markdown
Contributor

@andrzejj0 andrzejj0 commented Sep 12, 2022

Providing the possibility to provide the ruleSet via POM. As an extra which comes at a low cost, it will also be possible to provide just the global ignored versions via the maven.version.ignore property (property name chosen to be in line with similar properties for this plugin).

Added some unit tests where it didn't come at a great runtime cost, in other cases created integration tests to test as many mojos as possible where the change occurs.

On top of that, some very minor refactoring and cleanup.

As usual, kindly please review @slawekjaranowski

@andrzejj0 andrzejj0 force-pushed the issue-684-property-based-rules-ruleset branch from 73c09da to 3731a73 Compare September 12, 2022 11:04
Copy link
Copy Markdown
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

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

We have two methods loadRuleSet in DefaultVersionsHelper
one is called from constructor one from builder
there is logic for checking ignoredVersions and
there is logic for checking ignore Versions and creating / coping ruleSet depends on it ... it looks a little complicated

Maybe simply in order:

  • if ruleSet exist in plugin configuration use it
  • if rulesUri exist use it
  • always add ignoredVersions to ruleSet

Comment thread src/it/it-compare-dependencies-001/invoker.properties Outdated
Comment thread src/it/it-compare-dependencies-001/invoker.properties Outdated
Comment thread src/it/it-plugin-updates-report-issue-684-001/pom.xml Outdated
Comment thread src/site/apt/version-rules.apt.vm
@andrzejj0 andrzejj0 force-pushed the issue-684-property-based-rules-ruleset branch from 3731a73 to 4e2e521 Compare September 12, 2022 19:18
@andrzejj0
Copy link
Copy Markdown
Contributor Author

Remarks applied. Please re-review. :)

Comment thread src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java Outdated
Comment thread src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java Outdated
Comment thread src/site/apt/version-rules.apt.vm Outdated
Comment thread src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java Outdated
@andrzejj0 andrzejj0 force-pushed the issue-684-property-based-rules-ruleset branch from 4e2e521 to 3127ac3 Compare September 13, 2022 04:39
@andrzejj0
Copy link
Copy Markdown
Contributor Author

Again, applied all comments. Please review again.

@slawekjaranowski slawekjaranowski merged commit f0092d0 into mojohaus:master Sep 13, 2022
@slawekjaranowski
Copy link
Copy Markdown
Member

@ajarmoniuk Thanks - good job

@andrzejj0 andrzejj0 deleted the issue-684-property-based-rules-ruleset branch September 13, 2022 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Option to ignore plugin versions in pom.xml without need for Rules.xml

2 participants