[GitHub] [maven-enforcer] KroArtem opened a new pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem opened a new pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox

KroArtem opened a new pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86


   Following this checklist to help us incorporate your
   contribution quickly and easily:
   
    - [ ] 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.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] 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.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
          be performed on your pull request automatically.
   
   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.
   
    - [ ] 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)
   
    - [ ] 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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] rfscholte commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox

rfscholte commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r563653714



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()

Review comment:
       this will break backwards compatibility.
   
   Better to do something like
   
       public void setName(String name)
       {
         this.includes = Collections.singletonList(name);
         getLog().warning("property name is deprecated, replace with includes.")
       }
   
   This is just an example, please verify that the name still works.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r563658948



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()

Review comment:
       What's up with backwards compatibility? This rule was not released in any version (M4 was not released), there is no information about this rule on site https://maven.apache.org/enforcer/enforcer-rules/index.html
   
   From my point of view, this was specifically done to prevent breaking backward compatibility in future. Here is your comment as an example 😃 https://issues.apache.org/jira/browse/MENFORCER-338?focusedCommentId=17169274&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17169274




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] elharo commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

elharo commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r563677436



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()
     {
-        this.name = name;
+        return this.includes;
+    }
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     * *
+     * @see #setIncludes(List)
+     * @param theIncludes the includes to set
+     */
+    public void setIncludes( List<String> theIncludes )
+    {
+        this.includes = theIncludes;
+    }
+
+    private void createException() throws EnforcerRuleException
+    {
+        String message = getMessage();
+        String error = "Vendor " + SystemUtils.JAVA_VENDOR + " is in a list of banned vendors: " + excludes;
+        StringBuilder sb = new StringBuilder();
+        if ( message != null )
+        {
+            sb.append( message ).append( System.lineSeparator() );

Review comment:
       Just \n please. Exception messages aren't just for System.out.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r563658948



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()

Review comment:
       What's up with backwards compatibility? This rule was not released in any version (M4 was not released), there is no information about this rule on site https://maven.apache.org/enforcer/enforcer-rules/index.html
   
   From my point of view, this was specifically done to prevent breaking backward compatibility in future. Here is your comment as an example 😃 https://issues.apache.org/jira/browse/MENFORCER-338?focusedCommentId=17169274&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17169274




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] elharo commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

elharo commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r563677436



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()
     {
-        this.name = name;
+        return this.includes;
+    }
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     * *
+     * @see #setIncludes(List)
+     * @param theIncludes the includes to set
+     */
+    public void setIncludes( List<String> theIncludes )
+    {
+        this.includes = theIncludes;
+    }
+
+    private void createException() throws EnforcerRuleException
+    {
+        String message = getMessage();
+        String error = "Vendor " + SystemUtils.JAVA_VENDOR + " is in a list of banned vendors: " + excludes;
+        StringBuilder sb = new StringBuilder();
+        if ( message != null )
+        {
+            sb.append( message ).append( System.lineSeparator() );

Review comment:
       Just \n please. Exception messages aren't just for System.out.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] rfscholte commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

rfscholte commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r563653714



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()

Review comment:
       this will break backwards compatibility.
   
   Better to do something like
   
       public void setName(String name)
       {
         this.includes = Collections.singletonList(name);
         getLog().warning("property name is deprecated, replace with includes.")
       }
   
   This is just an example, please verify that the name still works.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] bmarwell commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

bmarwell commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r565072791



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()

Review comment:
       Probably just an oversight that this was never part of a release. Happens to all of us. :)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] bmarwell commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

bmarwell commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r565072990



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()
     {
-        this.name = name;
+        return this.includes;
+    }
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     * *
+     * @see #setIncludes(List)
+     * @param theIncludes the includes to set
+     */
+    public void setIncludes( List<String> theIncludes )
+    {
+        this.includes = theIncludes;
+    }
+
+    private void createException() throws EnforcerRuleException
+    {
+        String message = getMessage();
+        String error = "Vendor " + SystemUtils.JAVA_VENDOR + " is in a list of banned vendors: " + excludes;
+        StringBuilder sb = new StringBuilder();
+        if ( message != null )
+        {
+            sb.append( message ).append( System.lineSeparator() );

Review comment:
       @KroArtem I think this might be the only fix needed. Rest looks fine to 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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r565083783



##########
File path: enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
##########
@@ -31,40 +33,112 @@
  */
 public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule
 {
-    private String name;
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * Some examples are:
+     * <ul>
+     * <li><code>AdoptOpenJDK</code> prohibits vendor name AdoptOpenJDK </li>
+     * <li><code>Amazon</code> prohibits vendor name Amazon </li>
+     * </ul>
+     *
+     * @see #setExcludes(List)
+     * @see #getExcludes()
+     */
+    private List<String> excludes = null;
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     *
+     * @see #setIncludes(List)
+     * @see #getIncludes()
+     */
+    private List<String> includes = null;
 
     @Override
     public void execute( EnforcerRuleHelper helper ) throws EnforcerRuleException
     {
-        if ( !SystemUtils.JAVA_VENDOR.equals( name ) )
+        if ( excludes != null )
         {
-            String message = getMessage();
-            String error = "Vendor " + SystemUtils.JAVA_VENDOR + " did not match required vendor " + name;
-            StringBuilder sb = new StringBuilder();
-            if ( message != null )
+            if ( excludes.contains( SystemUtils.JAVA_VENDOR ) )
             {
-                sb.append( message ).append( System.lineSeparator() );
+                if ( includes != null )
+                {
+                    if ( !includes.contains( SystemUtils.JAVA_VENDOR ) )
+                    {
+                        createException();
+                    }
+                    return;
+                }
+                createException();
             }
-
-            sb.append( error );
-
-            throw new EnforcerRuleException( sb.toString() );
         }
     }
 
     /**
-     * Specify the required name. Some examples are:
-     * Name should be an exact match of the System Property java.vendor, which you can also see with mvn --version
+     * Gets the excludes.
      *
-     * <ul>
-     * <li><code>AdoptOpenJDK</code> enforces vendor name AdoptOpenJDK </li>
-     * <li><code>Amazon</code> enforces vendor name Amazon </li>
-     * </ul>
+     * @return the excludes
+     */
+    public List<String> getExcludes()
+    {
+        return this.excludes;
+    }
+
+    /**
+     * Specify the banned vendors. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * The rule will fail if vendor name matches any exclude, unless it also matches an
+     * include rule.
+     *
+     * @see #getExcludes()
+     * @param theExcludes the excludes to set
+     */
+    public void setExcludes( List<String> theExcludes )
+    {
+        this.excludes = theExcludes;
+    }
+
+    /**
+     * Gets the includes.
      *
-     * @param name the required name to set
+     * @return the includes
      */
-    public final void setName( String name )
+    public List<String> getIncludes()
     {
-        this.name = name;
+        return this.includes;
+    }
+
+    /**
+     * Specify the allowed vendor names. This should be an exact match of the System Property
+     * java.vendor, which you can also see with mvn --version. <br>
+     * Includes override the exclude rules.
+     * *
+     * @see #setIncludes(List)
+     * @param theIncludes the includes to set
+     */
+    public void setIncludes( List<String> theIncludes )
+    {
+        this.includes = theIncludes;
+    }
+
+    private void createException() throws EnforcerRuleException
+    {
+        String message = getMessage();
+        String error = "Vendor " + SystemUtils.JAVA_VENDOR + " is in a list of banned vendors: " + excludes;
+        StringBuilder sb = new StringBuilder();
+        if ( message != null )
+        {
+            sb.append( message ).append( System.lineSeparator() );

Review comment:
       ok, I'll fix it today. Just understood that documentation (`requireJavaVendor.apt.vm`) has to be updated too.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#issuecomment-770444673


   I still have one question, what should happen if I define a list of `includes` but current vendor doesn't satisfy this list? With this implementation it doesn't fail. However, it seem to be aligned with `bannedPlugins` [rule](https://maven.apache.org/enforcer/enforcer-rules/bannedPlugins.html), citing it:
   
   ```
   includes - a list of plugin artifacts to include. These are exceptions to the excludes. It is meant to allow wide exclusion rules with wildcards and fine tune using includes. If nothing has been excluded, then the includes have no effect. In otherwords, includes only subtract from artifacts that matched an exclude rule.
   ```
   
   That's the only question left so far.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#issuecomment-773508958


   @rfscholte , @bmarwell , @elharo , can I kindly ask you to review MR again?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#issuecomment-773563518


   > The docs that support this feature should be added in this PR as well.
   
   @elharo , what exactly do you mean? I've already made corresponding changes in .apt.vm files. Is there something that should be updated too?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] elharo commented on pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

elharo commented on pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#issuecomment-773597795


   Yes, those changes should be in this PR too.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#issuecomment-773604746


   I may be silly but these changes ARE in the PR right now, still don't get what else should be done. Here is where changes are: https://github.com/apache/maven-enforcer/pull/86/files#diff-b2cfcc9c03ff6b43bade73782c36c14b900b63cb39f57bea78a40a6cf62ae259R33


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] elharo commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

elharo commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r570550213



##########
File path: enforcer-rules/src/site/apt/requireJavaVendor.apt.vm
##########
@@ -53,7 +56,13 @@ Require Java Vendor
             <configuration>
               <rules>
                 <requireJavaVendor>
-                  <name>AdoptOpenJDK</name>
+                  <excludes>
+                    <exclude>Pivotal</exclude>

Review comment:
       are these the actual vendor names we would see?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] elharo commented on pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

elharo commented on pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#issuecomment-773597795


   Yes, those changes should be in this PR too.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] elharo commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

elharo commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r570550213



##########
File path: enforcer-rules/src/site/apt/requireJavaVendor.apt.vm
##########
@@ -53,7 +56,13 @@ Require Java Vendor
             <configuration>
               <rules>
                 <requireJavaVendor>
-                  <name>AdoptOpenJDK</name>
+                  <excludes>
+                    <exclude>Pivotal</exclude>

Review comment:
       are these the actual vendor names we would see?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#issuecomment-773508958






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-enforcer] KroArtem commented on a change in pull request #86: [MENFORCER-376] Add excludes/includes functionality to RequireJavaVendor rule

GitBox
In reply to this post by GitBox

KroArtem commented on a change in pull request #86:
URL: https://github.com/apache/maven-enforcer/pull/86#discussion_r581165543



##########
File path: enforcer-rules/src/site/apt/requireJavaVendor.apt.vm
##########
@@ -53,7 +56,13 @@ Require Java Vendor
             <configuration>
               <rules>
                 <requireJavaVendor>
-                  <name>AdoptOpenJDK</name>
+                  <excludes>
+                    <exclude>Pivotal</exclude>

Review comment:
       These are the names I took from jdk.dev
   
   My current mvn -v returns the following:
   
   ```
   Java version: 15.0.1, vendor: N/A, runtime: /usr/local/Cellar/openjdk/15.0.1/libexec/openjdk.jdk/Contents/Home
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


12