You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by GitBox <gi...@apache.org> on 2022/11/14 14:17:54 UTC

[GitHub] [johnzon] SingingBush opened a new pull request, #96: update maven plugins

SingingBush opened a new pull request, #96:
URL: https://github.com/apache/johnzon/pull/96

   building on the work done by @hboutemy in PR #93, and the javadoc fixes in PR #95, these changes fix some outstanding errors with Javadoc and aim to improve the build by making sure that maven plugins defined by the parent pom are not overridden while also updating maven plugins that are added specifically to Johnzon. In the case of the cobertura plugin, I've removed it entirely as it's been defunct for some time and Jacoco should be used instead. Both checkstyle and finbugs need further changes which can be in a future PR. I've amended the Github Action to now run maven verify as that includes additional stages that were not covered by package and were previously failing. I've also enabled javadoc linting as it no longer fails.


-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] rmannibucau commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021667525


##########
pom.xml:
##########
@@ -387,8 +433,7 @@
               <detectJavaApiLink>false</detectJavaApiLink>
               <detectLinks>false</detectLinks>
               <detectOfflineLinks>false</detectOfflineLinks>
-              <doclint>none</doclint>
-              <source>8</source>

Review Comment:
   right but same than https://github.com/apache/johnzon/pull/96#discussion_r1021642715 (in particular using a parent which is not only ours like apache parent which sometimes make other choices than the defaults)



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021662696


##########
.github/workflows/maven.yml:
##########
@@ -22,10 +22,7 @@ jobs:
           distribution: ${{ matrix.dist }}
           cache: 'maven'
 
-      - name: Maven Package
+      - name: Maven Verify
         env:
           MAVEN_OPTS: -Dmaven.artifact.threads=64
-        run: mvn -V package --no-transfer-progress
-
-      #- name: Maven javadoc
-      #  run: mvn javadoc:javadoc -Ddoclint=all --no-transfer-progress
+        run: mvn -V verify --no-transfer-progress

Review Comment:
   fixed in new commit



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] rmannibucau commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021602898


##########
.github/workflows/maven.yml:
##########
@@ -22,10 +22,7 @@ jobs:
           distribution: ${{ matrix.dist }}
           cache: 'maven'
 
-      - name: Maven Package
+      - name: Maven Verify
         env:
           MAVEN_OPTS: -Dmaven.artifact.threads=64
-        run: mvn -V package --no-transfer-progress
-
-      #- name: Maven javadoc
-      #  run: mvn javadoc:javadoc -Ddoclint=all --no-transfer-progress
+        run: mvn -V verify --no-transfer-progress

Review Comment:
   we don't need verify AFAIK so let's not misguide users/us with an irrelevant command in the project



##########
pom.xml:
##########
@@ -397,7 +442,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
-        <version>3.0.0-M3</version>
+        <!-- Version defined in Apache parent pom -->

Review Comment:
   same but can be upgraded ;)



##########
pom.xml:
##########
@@ -239,7 +286,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-site-plugin</artifactId>
-        <version>3.5.1</version>

Review Comment:
   same



##########
pom.xml:
##########
@@ -375,7 +421,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-javadoc-plugin</artifactId>
-        <version>3.2.0</version>
+        <!-- Version defined in Apache parent pom -->

Review Comment:
   same



##########
pom.xml:
##########
@@ -517,12 +523,11 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-site-plugin</artifactId>
-        <version>3.5.1</version>
+        <!-- Version defined in Apache parent pom -->

Review Comment:
   same



##########
pom.xml:
##########
@@ -103,7 +116,7 @@
         <plugin>
           <groupId>org.apache.maven.plugins</groupId>
           <artifactId>maven-shade-plugin</artifactId>
-          <version>3.2.3</version>
+          <!-- Version defined in Apache parent pom -->

Review Comment:
   let's keep it there to not be unstable due to "external" parent pom



##########
pom.xml:
##########
@@ -470,25 +494,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-enforcer-plugin</artifactId>
-        <version>3.0.0-M2</version>
-        <executions>
-          <execution>
-            <id>enforce-versions</id>
-            <goals>
-              <goal>enforce</goal>
-            </goals>
-            <configuration>
-              <rules>
-                <requireMavenVersion>
-                  <version>[3.3,)</version>
-                </requireMavenVersion>
-                <requireJavaVersion>
-                  <version>[${java-compile.version},)</version>
-                </requireJavaVersion>
-              </rules>
-            </configuration>
-          </execution>
-        </executions>
+        <!-- Version (and configuration) defined in Apache parent pom -->

Review Comment:
   same



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021618768


##########
johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbRecordTest.java:
##########
@@ -34,19 +34,19 @@ public class JsonbRecordTest {
 
     @Test
     public void roundTrip() {
-        final Record ref = new Record(119, "Santa");
+        final JsonbRecord ref = new JsonbRecord(119, "Santa");
         final String expectedJson = "{\"_name\":\"Santa\",\"age\":119}";
         assertEquals(expectedJson, jsonb.toJson(ref));
-        assertEquals(ref, jsonb.fromJson(expectedJson, Record.class));
+        assertEquals(ref, jsonb.fromJson(expectedJson, JsonbRecord.class));
     }
 
     @JohnzonRecord
-    public static class Record {
+    public static class JsonbRecord {

Review Comment:
   I can revert that if needed. I felt that the 'Record' naming was a little to close to the 'record' keyword



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on PR #96:
URL: https://github.com/apache/johnzon/pull/96#issuecomment-1313959040

   I think I've resolved all issues. requested re-review to to force pushing squashed commits. I also updated original comment to better reflect the current changes in the 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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] rmannibucau commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021680080


##########
pom.xml:
##########
@@ -164,16 +177,50 @@
             </execution>
           </executions>
         </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-checkstyle-plugin</artifactId>
+          <version>${checkstyle.version}</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>findbugs-maven-plugin</artifactId>
+          <!-- todo: migrate to spotbugs-maven-plugin as the findbugs plugin is no longer maintained -->
+          <version>3.0.5</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-pmd-plugin</artifactId>
+          <version>3.19.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>versions-maven-plugin</artifactId>
+          <version>2.13.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>taglist-maven-plugin</artifactId>
+          <version>3.0.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changelog-plugin</artifactId>
+          <version>2.3</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changes-plugin</artifactId>
+          <version>2.12.1</version>
+        </plugin>
       </plugins>
     </pluginManagement>
+
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <source>${java-compile.version}</source>

Review Comment:
   it is ok for me to use the property with the default name, no issue there but ensure the wiring is explicit please (funnily I used the exact same reasonning so hope both can converge ;))



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021664833


##########
pom.xml:
##########
@@ -103,7 +116,7 @@
         <plugin>
           <groupId>org.apache.maven.plugins</groupId>
           <artifactId>maven-shade-plugin</artifactId>
-          <version>3.2.3</version>
+          <!-- Version defined in Apache parent pom -->

Review Comment:
   fixed in new commit. plugin versions are defined under _pluginManagement_ in main pom



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021638210


##########
pom.xml:
##########
@@ -164,16 +177,50 @@
             </execution>
           </executions>
         </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-checkstyle-plugin</artifactId>
+          <version>${checkstyle.version}</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>findbugs-maven-plugin</artifactId>
+          <!-- todo: migrate to spotbugs-maven-plugin as the findbugs plugin is no longer maintained -->
+          <version>3.0.5</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-pmd-plugin</artifactId>
+          <version>3.19.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>versions-maven-plugin</artifactId>
+          <version>2.13.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>taglist-maven-plugin</artifactId>
+          <version>3.0.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changelog-plugin</artifactId>
+          <version>2.3</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changes-plugin</artifactId>
+          <version>2.12.1</version>
+        </plugin>
       </plugins>
     </pluginManagement>
+
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <source>${java-compile.version}</source>

Review Comment:
   the pom in the root of the project has:
   
   ```
       <maven.compiler.source>1.8</maven.compiler.source>
       <maven.compiler.target>1.8</maven.compiler.target>
   ```
   
   which are standard properties that maven-compiler-plugin respects. There's no need to set them explicitly in the configuration if defined in properties.



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021627910


##########
.github/workflows/maven.yml:
##########
@@ -22,10 +22,7 @@ jobs:
           distribution: ${{ matrix.dist }}
           cache: 'maven'
 
-      - name: Maven Package
+      - name: Maven Verify
         env:
           MAVEN_OPTS: -Dmaven.artifact.threads=64
-        run: mvn -V package --no-transfer-progress
-
-      #- name: Maven javadoc
-      #  run: mvn javadoc:javadoc -Ddoclint=all --no-transfer-progress
+        run: mvn -V verify --no-transfer-progress

Review Comment:
   I thought verify was needed to ensure plugins like checkstyle and the javadoc linting kick in. If they are running as part of package then it can be reverted



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] rmannibucau merged pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
rmannibucau merged PR #96:
URL: https://github.com/apache/johnzon/pull/96


-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] rmannibucau commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021605409


##########
pom.xml:
##########
@@ -164,16 +177,50 @@
             </execution>
           </executions>
         </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-checkstyle-plugin</artifactId>
+          <version>${checkstyle.version}</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>findbugs-maven-plugin</artifactId>
+          <!-- todo: migrate to spotbugs-maven-plugin as the findbugs plugin is no longer maintained -->
+          <version>3.0.5</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-pmd-plugin</artifactId>
+          <version>3.19.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>versions-maven-plugin</artifactId>
+          <version>2.13.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>taglist-maven-plugin</artifactId>
+          <version>3.0.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changelog-plugin</artifactId>
+          <version>2.3</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changes-plugin</artifactId>
+          <version>2.12.1</version>
+        </plugin>
       </plugins>
     </pluginManagement>
+
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <source>${java-compile.version}</source>

Review Comment:
   why? should stay in the project IMHO



##########
johnzon-osgi/pom.xml:
##########
@@ -29,6 +29,10 @@
   <name>Johnzon :: Support for OSGI Jaxrs Whiteboard</name>
   <packaging>bundle</packaging>
 
+  <properties>
+    <maven.javadoc.skip>true</maven.javadoc.skip>

Review Comment:
   guess it will break the release no?



##########
johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbRecordTest.java:
##########
@@ -34,19 +34,19 @@ public class JsonbRecordTest {
 
     @Test
     public void roundTrip() {
-        final Record ref = new Record(119, "Santa");
+        final JsonbRecord ref = new JsonbRecord(119, "Santa");
         final String expectedJson = "{\"_name\":\"Santa\",\"age\":119}";
         assertEquals(expectedJson, jsonb.toJson(ref));
-        assertEquals(ref, jsonb.fromJson(expectedJson, Record.class));
+        assertEquals(ref, jsonb.fromJson(expectedJson, JsonbRecord.class));
     }
 
     @JohnzonRecord
-    public static class Record {
+    public static class JsonbRecord {

Review Comment:
   why was it needed?



##########
pom.xml:
##########
@@ -387,8 +433,7 @@
               <detectJavaApiLink>false</detectJavaApiLink>
               <detectLinks>false</detectLinks>
               <detectOfflineLinks>false</detectOfflineLinks>
-              <doclint>none</doclint>
-              <source>8</source>

Review Comment:
   source needed I think



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] rmannibucau commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021625991


##########
johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbRecordTest.java:
##########
@@ -34,19 +34,19 @@ public class JsonbRecordTest {
 
     @Test
     public void roundTrip() {
-        final Record ref = new Record(119, "Santa");
+        final JsonbRecord ref = new JsonbRecord(119, "Santa");
         final String expectedJson = "{\"_name\":\"Santa\",\"age\":119}";
         assertEquals(expectedJson, jsonb.toJson(ref));
-        assertEquals(ref, jsonb.fromJson(expectedJson, Record.class));
+        assertEquals(ref, jsonb.fromJson(expectedJson, JsonbRecord.class));
     }
 
     @JohnzonRecord
-    public static class Record {
+    public static class JsonbRecord {

Review Comment:
   yes, was the intent - and to be honest it is not lowercase just to run well on later java versions ;) - cause it is supposed to mimic a `record`



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021630047


##########
johnzon-osgi/pom.xml:
##########
@@ -29,6 +29,10 @@
   <name>Johnzon :: Support for OSGI Jaxrs Whiteboard</name>
   <packaging>bundle</packaging>
 
+  <properties>
+    <maven.javadoc.skip>true</maven.javadoc.skip>

Review Comment:
   yes the osgi properties were causing a real problem for javadoc. I tried excluding all _package-info.java_ but didn't get it working so decided just to skip javadoc on that module



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021659592


##########
pom.xml:
##########
@@ -387,8 +433,7 @@
               <detectJavaApiLink>false</detectJavaApiLink>
               <detectLinks>false</detectLinks>
               <detectOfflineLinks>false</detectOfflineLinks>
-              <doclint>none</doclint>
-              <source>8</source>

Review Comment:
   Javadoc plugin will use _maven.compiler.source_, see: https://maven.apache.org/plugins/maven-javadoc-plugin/javadoc-mojo.html



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] rmannibucau commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021642715


##########
pom.xml:
##########
@@ -164,16 +177,50 @@
             </execution>
           </executions>
         </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-checkstyle-plugin</artifactId>
+          <version>${checkstyle.version}</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>findbugs-maven-plugin</artifactId>
+          <!-- todo: migrate to spotbugs-maven-plugin as the findbugs plugin is no longer maintained -->
+          <version>3.0.5</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-pmd-plugin</artifactId>
+          <version>3.19.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>versions-maven-plugin</artifactId>
+          <version>2.13.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>taglist-maven-plugin</artifactId>
+          <version>3.0.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changelog-plugin</artifactId>
+          <version>2.3</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changes-plugin</artifactId>
+          <version>2.12.1</version>
+        </plugin>
       </plugins>
     </pluginManagement>
+
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <source>${java-compile.version}</source>

Review Comment:
   have to admit I prefer the configuration to be colocalised instead of split accross section, it enables a better readability in editors and avoids to scroll to get it (hover gives the value to be concrete). The other advantage for newbies is it avoids to know all conventions and check the doc/plugin.xml to get the defaults. wdyt?



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [johnzon] SingingBush commented on a diff in pull request #96: update maven plugins

Posted by GitBox <gi...@apache.org>.
SingingBush commented on code in PR #96:
URL: https://github.com/apache/johnzon/pull/96#discussion_r1021670047


##########
pom.xml:
##########
@@ -164,16 +177,50 @@
             </execution>
           </executions>
         </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-checkstyle-plugin</artifactId>
+          <version>${checkstyle.version}</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>findbugs-maven-plugin</artifactId>
+          <!-- todo: migrate to spotbugs-maven-plugin as the findbugs plugin is no longer maintained -->
+          <version>3.0.5</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-pmd-plugin</artifactId>
+          <version>3.19.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>versions-maven-plugin</artifactId>
+          <version>2.13.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>taglist-maven-plugin</artifactId>
+          <version>3.0.0</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changelog-plugin</artifactId>
+          <version>2.3</version>
+        </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-changes-plugin</artifactId>
+          <version>2.12.1</version>
+        </plugin>
       </plugins>
     </pluginManagement>
+
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <source>${java-compile.version}</source>

Review Comment:
   I've made a habit of specifying it in the property format on all maven projects I work on. Done that way for years and although there is some knowledge newcomers may need, it's something worth knowing, especially as that property is now used by numerous plugins (even non-apache). I can put it back to being explicit but would at least use the standard property key.



-- 
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: dev-unsubscribe@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org