[GitHub] [shiro] bmarwell commented on a diff in pull request #574: [SHIRO-900] Refactor and update pom.xml

[GitBox <gi...@apache.org>]

bmarwell commented on code in PR #574:
URL: https://github.com/apache/shiro/pull/574#discussion_r1055571725


##########
pom.xml:
##########
@@ -543,6 +548,12 @@
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-compiler-plugin</artifactId>
                 <version>3.8.0</version>
+                <configuration>
+                    <compilerArgs>
+                        <arg>-Xlint:deprecation</arg>
+                        <arg>-Xlint:unchecked</arg>
+                    </compilerArgs>
+                </configuration>

Review Comment:
   what about maven.compiler.showWarnings (property) or showWarnings (configuration item)? Is it set, too?



##########
pom.xml:
##########
@@ -330,13 +333,15 @@
                             <exclude>**/spring.factories</exclude>
                             <exclude>**/spring.provides</exclude>
                             <exclude>**/*.iml</exclude>
+                            <exclude>**/nb-configuration.xml</exclude>
+                            <exclude>**/faces-config.NavData</exclude>

Review Comment:
   This could also be in the `.gitignore` file.



##########
pom.xml:
##########
@@ -1462,6 +1474,8 @@
                         <exclude>**/spring.factories</exclude>
                         <exclude>**/spring.provides</exclude>
                         <exclude>**/*.iml</exclude>
+                        <exclude>**/nb-configuration.xml</exclude>
+                        <exclude>**/faces-config.NavData</exclude>

Review Comment:
   Same as above



##########
pom.xml:
##########
@@ -1575,6 +1589,16 @@
                 </pluginManagement>
             </build>
         </profile>
+        <profile>
+            <id>jdk11plus</id>
+            <activation>
+                <jdk>[11,)</jdk>
+            </activation>
+            <properties>
+                <failsafe.argLine>--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.net=ALL-UNNAMED</failsafe.argLine>
+                <surefire.argLine>-Xshare:off ${failsafe.argLine}</surefire.argLine>

Review Comment:
   -1, `-Xshare:off` will not work on all JDKs (like non-hotspot-JDKs as OpenJ9 which comes with IBM Semeru). Either additionally check for a hotspot JDK or just leave it out. It should not be required anyway.



##########
pom.xml:
##########
@@ -577,6 +588,9 @@
                                 <excludes>
                                     <exclude>**/main/**/samples/**</exclude>
                                 </excludes>
+                                <address>localhost</address>
+                                <port>${jacocoPort}</port>
+                                <propertyName>jacocoAgent</propertyName>

Review Comment:
   Why is this needed? Why was it not needed before?



##########
pom.xml:
##########
@@ -1575,6 +1589,16 @@
                 </pluginManagement>
             </build>
         </profile>
+        <profile>
+            <id>jdk11plus</id>
+            <activation>
+                <jdk>[11,)</jdk>
+            </activation>
+            <properties>
+                <failsafe.argLine>--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.net=ALL-UNNAMED</failsafe.argLine>

Review Comment:
   As far as I can see, this should not be needed anymore. Even then, we already have this in the guice projects so it should not live here for ALL modules. Only those which need 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: commits-unsubscribe@shiro.apache.org

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