You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/27 21:18:59 UTC
[GitHub] [maven-enforcer] psiroky opened a new pull request, #205: [MENFORCER-431] Introduce property to skip specific rules
psiroky opened a new pull request, #205:
URL: https://github.com/apache/maven-enforcer/pull/205
I am creating this draft PR to discuss couple of points. This is not yet ready merge. See the inline comments.
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MENFORCER) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[MENFORCER-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MENFORCER-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] You have run the integration tests successfully (`mvn -Prun-its clean verify`).
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [x] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057973498
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
+ /**
+ * Array of Strings that matches the EnforcerRules to skip.
+ *
+ * @since 3.2.0
+ */
+ @Parameter(required = false, property = "enforcer.skipRules")
+ private String[] commandLineRulesToSkip;
Review Comment:
Thanks for the additional info. I guess these are a bit specific since users likely won't use them directly in pom.xml config section, but rather via a property.
One more thing I am not sure about is the property name. `skipRules` may suggest that it is a boolean property, which is not the case here. I could not come up with a better name
* `rulesToSkip` seems weird as well
* `skippedRules` - not sure about the past tense?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] slawekjaranowski commented on pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#issuecomment-1366974598
@psiroky #207 - merged
Please look.
I will have next changes, so wait for you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce option to skip specific rules via custom property
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1058896974
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
+ /**
+ * Array of Strings that matches the EnforcerRules to skip.
+ *
+ * @since 3.2.0
+ */
+ @Parameter(required = false, property = "enforcer.skipRules")
+ private String[] commandLineRulesToSkip;
Review Comment:
I used the `skipRules` in the end, since it seems to better match the `rules` one to include rules. And it is also shorter / easier to write by hand, which I think is a plus since it is supposed to be used from cmd line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on pull request #205: [MENFORCER-431] Introduce option to skip specific rules via custom property
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#issuecomment-1367253679
@slawekjaranowski I believe this should be now ready for review. Please take another look when you get a chance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] slawekjaranowski commented on a diff in pull request #205: [MENFORCER-431] Introduce option to skip specific rules via custom property
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1058946939
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -282,6 +288,22 @@ private Optional<PlexusConfiguration> createRulesFromCommandLineOptions() {
return Optional.of(configuration);
}
+ /**
+ * Filter our (remove) rules that have been specifically skipped via additional configuration.
+ *
+ * @param allRules list of enforcer rules to go through and filter
+ *
+ * @return list of filtered rules
+ */
+ private List<EnforcerRuleDesc> filterOutSkippedRules(List<EnforcerRuleDesc> allRules) {
+ if (rulesToSkip == null || rulesToSkip.length == 0) {
+ return allRules;
+ }
+ return allRules.stream()
+ .filter(ruleDesc -> !ArrayUtils.contains(rulesToSkip, ruleDesc.getName()))
Review Comment:
We can use `List<String>` as parameter type, then external library will be not needed 😄
There aare more parameter as arrays ... we can improve in next PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057920952
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -134,9 +147,7 @@ public void execute() throws MojoExecutionException {
Log log = this.getLog();
PluginParameterExpressionEvaluator evaluator = new PluginParameterExpressionEvaluator(session, mojoExecution);
- if (commandLineRules != null && commandLineRules.length > 0) {
- this.rules = createRulesFromCommandLineOptions();
- }
+ this.rules = determineRulesToExecute();
Review Comment:
I am not the biggest fan of this re-assignment to the instance field, but this is the current way of doing it, so I left it for now.
I could potentially just create a local variable with the updated list and leave the fields be. This could be however considered a breaking change since the `getRules()` would then be returning different list than before (updated vs original).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#issuecomment-1366742601
@slawekjaranowski thanks for the heads-up. That seems to change things quite a bit, so I will probably wait until it gets merged and then re-do my changes on top of that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce option to skip specific rules via custom property
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1058955173
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -282,6 +288,22 @@ private Optional<PlexusConfiguration> createRulesFromCommandLineOptions() {
return Optional.of(configuration);
}
+ /**
+ * Filter our (remove) rules that have been specifically skipped via additional configuration.
+ *
+ * @param allRules list of enforcer rules to go through and filter
+ *
+ * @return list of filtered rules
+ */
+ private List<EnforcerRuleDesc> filterOutSkippedRules(List<EnforcerRuleDesc> allRules) {
+ if (rulesToSkip == null || rulesToSkip.length == 0) {
+ return allRules;
+ }
+ return allRules.stream()
+ .filter(ruleDesc -> !ArrayUtils.contains(rulesToSkip, ruleDesc.getName()))
Review Comment:
Ah, I thought we need to use `String[]`, since it is also being used for `commandLineRules` Just tried though and it lists are working as well, which is definitely nicer.
I can create additional PR to get this cleaned up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057922405
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
+ /**
+ * Array of Strings that matches the EnforcerRules to skip.
+ *
+ * @since 3.2.0
+ */
+ @Parameter(required = false, property = "enforcer.skipRules")
+ private String[] commandLineRulesToSkip;
Review Comment:
I don't really like the name here, since this is just a property value, which may not be supplied via command line at all, but instead via a property in pom.xml. I understand that the main use case is likely to override the rules via cmd line, but still seems a bit weird.
On the other hand, since the other property (`commandLineRules`) is already using this name it only makes sense to be consistent. So either rename both or none (hopefully renaming the field would not be considered a breaking change since it is `private`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057972238
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -134,9 +147,7 @@ public void execute() throws MojoExecutionException {
Log log = this.getLog();
PluginParameterExpressionEvaluator evaluator = new PluginParameterExpressionEvaluator(session, mojoExecution);
- if (commandLineRules != null && commandLineRules.length > 0) {
- this.rules = createRulesFromCommandLineOptions();
- }
+ this.rules = determineRulesToExecute();
Review Comment:
Yeah, maybe I am too paranoid. I was thinking if someone is doing e.g. `new EnforcerMojo().getRules()` then removing the getter would break that. Good question is how much sense does that even make and if this is anything that we should care about.
If you agree I would remove the getter then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] slawekjaranowski merged pull request #205: [MENFORCER-431] Introduce option to skip specific rules via custom property
Posted by GitBox <gi...@apache.org>.
slawekjaranowski merged PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce option to skip specific rules via custom property
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1058960968
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -282,6 +288,22 @@ private Optional<PlexusConfiguration> createRulesFromCommandLineOptions() {
return Optional.of(configuration);
}
+ /**
+ * Filter our (remove) rules that have been specifically skipped via additional configuration.
+ *
+ * @param allRules list of enforcer rules to go through and filter
+ *
+ * @return list of filtered rules
+ */
+ private List<EnforcerRuleDesc> filterOutSkippedRules(List<EnforcerRuleDesc> allRules) {
+ if (rulesToSkip == null || rulesToSkip.length == 0) {
+ return allRules;
+ }
+ return allRules.stream()
+ .filter(ruleDesc -> !ArrayUtils.contains(rulesToSkip, ruleDesc.getName()))
Review Comment:
Here is a small follow up PR, replace the arrays with lists: https://github.com/apache/maven-enforcer/pull/210
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057921554
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -242,22 +253,59 @@ public void execute() throws MojoExecutionException {
// CHECKSTYLE_ON: LineLength
}
- private EnforcerRule[] createRulesFromCommandLineOptions() throws MojoExecutionException {
- EnforcerRule[] rules = new EnforcerRule[commandLineRules.length];
- for (int i = 0; i < commandLineRules.length; i++) {
- String rule = commandLineRules[i];
- if (!rule.contains(".")) {
- rule = getClass().getPackage().getName() + "." + Character.toUpperCase(rule.charAt(0))
- + rule.substring(1);
- }
+ private EnforcerRule[] determineRulesToExecute() throws MojoExecutionException {
+ if (isNullOrEmpty(commandLineRules) && isNullOrEmpty(commandLineRulesToSkip)) {
+ return this.rules;
+ }
+ List<EnforcerRule> includedRules;
+ if (commandLineRules != null && commandLineRules.length > 0) {
+ includedRules = new ArrayList<>();
+ for (String commandLineRule : commandLineRules) {
+ String rule = commandLineRule;
+ if (!rule.contains(".")) {
+ rule = ruleNameToFullyQualifiedClassName(rule);
+ }
- try {
- rules[i] = (EnforcerRule) Class.forName(rule).newInstance();
Review Comment:
I am wondering if this is correct. If there is a specific configuration for any of the rules coming from e.g. `pom.xml`, this would effectively replace it with the default since we are instantiating new instance here.
Should we maybe check this specific rule is in the `rules` field and if so then just re-use it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057922405
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
+ /**
+ * Array of Strings that matches the EnforcerRules to skip.
+ *
+ * @since 3.2.0
+ */
+ @Parameter(required = false, property = "enforcer.skipRules")
+ private String[] commandLineRulesToSkip;
Review Comment:
I don't really like the field name here, since this represents a property value, which may not be supplied via command line at all, but instead via a property in pom.xml. I understand that the main use case is likely to override the rules via cmd line, but still seems a bit weird.
On the other hand, since the other property (`commandLineRules`) is already using this name it only makes sense to be consistent. So either rename both or none (hopefully renaming the field would not be considered a breaking change since it is `private`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] slawekjaranowski commented on a diff in pull request #205: [MENFORCER-431] Introduce option to skip specific rules via custom property
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1058959931
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -282,6 +288,22 @@ private Optional<PlexusConfiguration> createRulesFromCommandLineOptions() {
return Optional.of(configuration);
}
+ /**
+ * Filter our (remove) rules that have been specifically skipped via additional configuration.
+ *
+ * @param allRules list of enforcer rules to go through and filter
+ *
+ * @return list of filtered rules
+ */
+ private List<EnforcerRuleDesc> filterOutSkippedRules(List<EnforcerRuleDesc> allRules) {
+ if (rulesToSkip == null || rulesToSkip.length == 0) {
+ return allRules;
+ }
+ return allRules.stream()
+ .filter(ruleDesc -> !ArrayUtils.contains(rulesToSkip, ruleDesc.getName()))
Review Comment:
https://maven.apache.org/guides/mini/guide-configuring-plugins.html#Mapping_Collections_and_Arrays
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] slawekjaranowski commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1058252905
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
+ /**
+ * Array of Strings that matches the EnforcerRules to skip.
+ *
+ * @since 3.2.0
+ */
+ @Parameter(required = false, property = "enforcer.skipRules")
+ private String[] commandLineRulesToSkip;
Review Comment:
`skipRules` and `rulesToSkip` - are both ok for me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] slawekjaranowski commented on pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#issuecomment-1366715769
@psiroky please look at #207 - it can impact on your code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] psiroky commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057922768
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
Review Comment:
The property name `rules` seems wrong. I would expect the `enforcer.` prefix here.
We can't really rename the property at this point since that would be a breaking change, but maybe we could deprecate it and introduce new one named `enforcer.rules` which would be the replacement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] slawekjaranowski commented on a diff in pull request #205: [MENFORCER-431] Introduce property to skip specific rules
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #205:
URL: https://github.com/apache/maven-enforcer/pull/205#discussion_r1057954678
##########
maven-enforcer-plugin/src/it/projects/cli-skip-rules/invoker.properties:
##########
@@ -0,0 +1,18 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+invoker.goals = validate -Denforcer.skipRules=alwaysFail
Review Comment:
- please add verify script yo be sure that rule `alwaysPass` was executed
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
+ /**
+ * Array of Strings that matches the EnforcerRules to skip.
+ *
+ * @since 3.2.0
+ */
+ @Parameter(required = false, property = "enforcer.skipRules")
+ private String[] commandLineRulesToSkip;
Review Comment:
field name determinate configuration item for plugin, but can be changed by `@Parameter(name=...)`
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -134,9 +147,7 @@ public void execute() throws MojoExecutionException {
Log log = this.getLog();
PluginParameterExpressionEvaluator evaluator = new PluginParameterExpressionEvaluator(session, mojoExecution);
- if (commandLineRules != null && commandLineRules.length > 0) {
- this.rules = createRulesFromCommandLineOptions();
- }
+ this.rules = determineRulesToExecute();
Review Comment:
technically plugin doesn't need getters and setters - I see that `getRules` is not used
##########
maven-enforcer-plugin/src/main/java/org/apache/maven/plugins/enforcer/EnforceMojo.java:
##########
@@ -115,6 +120,14 @@ public class EnforceMojo extends AbstractMojo {
@Parameter(required = false, property = "rules")
private String[] commandLineRules;
Review Comment:
deprecated will be good idea - separate issue and PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org