sergehuber commented on code in PR #764:
URL: https://github.com/apache/unomi/pull/764#discussion_r3315933552


##########
.github/workflows/unomi-ci-build-tests.yml:
##########
@@ -31,7 +31,7 @@ jobs:
         sudo apt-get install -y graphviz
         dot -V
     - name: Build and Unit tests
-      run: mvn -U -ntp -e clean install
+      run: ./build.sh --ci

Review Comment:
   Valid point. `build.sh` runs `mvn clean` then `mvn install` as two separate 
JVM invocations rather than a single `mvn clean install`. The two-phase 
approach is there for developer UX (progress output between phases) but does 
add overhead in CI. A single-invocation CI fast-path is a worthwhile follow-up 
— out of scope for this hardening PR but worth tracking separately.



##########
setenv.sh:
##########
@@ -17,8 +17,13 @@
 #    limitations under the License.
 #
 
################################################################################
-export UNOMI_VERSION=`mvn 
org.apache.maven.plugins:maven-help-plugin:2.1.1:evaluate 
-Dexpression=project.version | grep -Ev '(^\[|Download\w+:)'`
-echo Detected project version=$UNOMI_VERSION
+# Quiet evaluate: avoid capturing Maven download lines into the environment 
(breaks CI with ARG_MAX).
+export UNOMI_VERSION="$(mvn -B -q -DforceStdout help:evaluate 
-Dexpression=project.version -DinteractiveMode=false 2>/dev/null)"
+if [ -z "$UNOMI_VERSION" ]; then
+    echo "Failed to detect project version from Maven" >&2
+    exit 1
+fi

Review Comment:
   `2>/dev/null` was added to prevent Maven's progress noise from bleeding into 
the captured version string (the old code piped stdout through `grep` for the 
same reason). You're right that it hides the actual error on failure; capturing 
stderr to a temp file and printing it when the version is empty would be a 
clean improvement. Noted as a follow-up.



-- 
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: [email protected]

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

Reply via email to