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

Reply via email to