Copilot commented on code in PR #349:
URL:
https://github.com/apache/maven-antrun-plugin/pull/349#discussion_r3690059636
##########
src/main/java/org/apache/maven/plugins/antrun/AntRunMojo.java:
##########
@@ -461,9 +461,13 @@ public void copyProperties(Project antProject,
MavenProject mavenProject) {
for (Map.Entry<String, Object> entry : antProps.entrySet()) {
String key = entry.getKey();
- if (mavenProperties.getProperty(key) != null) {
- getLog().warn("Ant property '" + key + "=" +
mavenProperties.getProperty(key)
- + "' clashes with an existing Maven property, SKIPPING
this Ant property propagation.");
+ String mavenValue = mavenProperties.getProperty(key);
+ if (mavenValue != null) {
+ if (!mavenValue.equals(entry.getValue())) {
+ getLog().info("Ant property '" + key + "=" +
entry.getValue()
+ + "' clashes with an existing Maven property value
'" + mavenValue
+ + "', SKIPPING this Ant property propagation.");
+ }
continue;
}
Review Comment:
`entry.getValue()` is an `Object`, but the comparison uses
`mavenValue.equals(entry.getValue())`. If Ant stores a non-`String` value whose
`toString()` matches the Maven value (and you later propagate using
`toString()`), this will still log an INFO “clash” even though the effective
string values are equal. Converting once to a `String` makes comparison and
logging consistent with the actual propagation behavior.
##########
src/main/java/org/apache/maven/plugins/antrun/AntRunMojo.java:
##########
@@ -461,9 +461,13 @@ public void copyProperties(Project antProject,
MavenProject mavenProject) {
for (Map.Entry<String, Object> entry : antProps.entrySet()) {
String key = entry.getKey();
- if (mavenProperties.getProperty(key) != null) {
- getLog().warn("Ant property '" + key + "=" +
mavenProperties.getProperty(key)
- + "' clashes with an existing Maven property, SKIPPING
this Ant property propagation.");
+ String mavenValue = mavenProperties.getProperty(key);
+ if (mavenValue != null) {
+ if (!mavenValue.equals(entry.getValue())) {
+ getLog().info("Ant property '" + key + "=" +
entry.getValue()
+ + "' clashes with an existing Maven property value
'" + mavenValue
+ + "', SKIPPING this Ant property propagation.");
+ }
Review Comment:
This change is primarily about reducing build output noise (Issue #348), but
there’s no test asserting the new logging behavior (no output when values
match; INFO only when values differ; no WARN spam). Consider adding an
integration test (maven-invoker `verify.bsh`) that inspects `build.log` to
prevent regressions in future refactors.
--
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]
