gnodet commented on code in PR #11857:
URL: https://github.com/apache/maven/pull/11857#discussion_r3284357890
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/PathModularizationCache.java:
##########
@@ -189,10 +189,12 @@ Optional<String>
warningForFilenameBasedAutomodules(Collection<Path> modulePaths
return Optional.empty();
}
String lineSeparator = System.lineSeparator();
+ String fileSeparator = lineSeparator + " - ";
var joiner = new StringJoiner(
- lineSeparator + " - ",
- "Filename-based automodules detected on the module path: " +
lineSeparator + " - ",
- lineSeparator + "Please don't publish this project to a public
artifact repository.");
+ fileSeparator,
+ "Filename-based automodules detected on the module path: " +
fileSeparator,
+ lineSeparator + "This project may not work in applications
that do not use "
+ + "these dependencies with exactly the same filenames
as listed above.");
automodulesDetected.forEach(joiner::add);
Review Comment:
The new suffix is fairly long and may be hard to parse in a log. Consider
shortening it slightly, e.g.:
```suggestion
lineSeparator + "This project may not work if consumers use "
+ "these dependencies with different filenames.");
```
The original phrasing ("do not use these dependencies with exactly the same
filenames as listed above") requires a double-negative mental parse ("not ...
exactly the same"). "Different filenames" says the same thing more directly.
Feel free to ignore if you prefer the current wording.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/PathModularization.java:
##########
@@ -236,7 +236,7 @@ public JavaPathType getPathType() {
/**
* If the module has no name, adds the filename of the JAR file in the
given collection.
* This method should be invoked for dependencies placed on {@link
JavaPathType#MODULES}
- * for preparing a warning asking to not deploy the build artifact on a
public repository.
+ * for preparing a warning saying that this project may fail to resolve
these modules.
Review Comment:
Nit: the Javadoc on this method still says "for preparing a warning asking
to not deploy the build artifact on a public repository", which no longer
matches what the warning actually says. Should be updated to stay consistent
with the new message.
```suggestion
* for preparing a warning saying that this project may fail to resolve
these modules.
```
--
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]
