On Fri, 13 Aug 2021 at 03:05, Matt Juntunen <matt.a.juntu...@gmail.com> wrote:
> Hi Alex, > > Thanks for catching these. Do you consider these a deal breaker for this > rc? > > -Matt > The issue with the site being downloaded is just annoying. The last numbers release had that issue too. All the multi-module builds have this issue. In use the common goal is 'mvn install' and this will not require the site lifecycle. The wrong year in the NOTICE may be a problem. Someone else can confirm. Here is what I found in a further review: Verified the source and binary distributions checksums and ASC signatures. Builds from the tag: Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f) Maven home: /usr/local/apache-maven-3.6.3 Java version: 11.0.12, vendor: Eclipse Foundation, runtime: /Library/Java/JavaVirtualMachines/temurin-11.jdk/Contents/Home Default locale: en_GB, platform encoding: UTF-8 OS name: "mac os x", version: "11.5", arch: "x86_64", family: "mac" mvn clean install site site:stage Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f) Maven home: /usr/local/apache-maven-3.6.3 Java version: 1.8.0_301, vendor: Oracle Corporation, runtime: /Library/Java/JavaVirtualMachines/jdk1.8.0_301.jdk/Contents/Home/jre Default locale: en_US, platform encoding: UTF-8 OS name: "mac os x", version: "10.16", arch: "x86_64", family: "mac" mvn clean install site site:stage Builds from the source distribution: Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f) Maven home: /usr/local/apache-maven-3.6.3 Java version: 1.8.0_301, vendor: Oracle Corporation, runtime: /Library/Java/JavaVirtualMachines/jdk1.8.0_301.jdk/Contents/Home/jre Default locale: en_US, platform encoding: UTF-8 OS name: "mac os x", version: "10.16", arch: "x86_64", family: "mac" mvn package Reports: There are a lot of PMD issues. Some could be removed just by changing a few rules. Some are simple fixes such as use of final members or variables. Others are genuine issues such as the use of confusing ternary [1] and assignments in operands [2]. This should be reviewed going forward. This has method level synchronization and uses a plain HashMap: org/apache/commons/geometry/io/core/BoundaryIOManager PMD flags this and recommends ConcurrentHashMap. It does not seem to require synchronization in the entire method for some of the methods. Test coverage is excellent but I found some items that should be covered and are not. No coverage in some public methods: org.apache.commons.geometry.euclidean.twod > EmbeddedTreeLineSubset org.apache.commons.geometry.spherical.twod > AbstractGreatArcConnector.ConnectableGreatArc org.apache.commons.geometry.spherical.twod > GreatArc org.apache.commons.geometry.spherical.twod > GreatCircleSubset No coverage of a thrown exception: org.apache.commons.geometry.euclidean.twod.path > LinePath.Builder.append Alex [1] https://pmd.github.io/pmd-6.36.0/pmd_rules_java_codestyle.html#confusingternary [2] https://pmd.github.io/pmd-6.36.0/pmd_rules_java_errorprone.html#assignmentinoperand