mthmulders commented on code in PR #868:
URL: https://github.com/apache/maven/pull/868#discussion_r1019955847
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -64,23 +65,36 @@ public MultiModuleCollectionStrategy( ModelLocator
modelLocator, ProjectsSelecto
@Override
public List<MavenProject> collectProjects( MavenExecutionRequest request )
throws ProjectBuildingException
{
- File moduleProjectPomFile = getMultiModuleProjectPomFile( request );
- List<File> files = Collections.singletonList(
moduleProjectPomFile.getAbsoluteFile() );
+ Optional<File> moduleProjectPomFile = getMultiModuleProjectPomFile(
request );
+ File absoluteFile = moduleProjectPomFile.orElse( request.getPom()
).getAbsoluteFile();
+ List<File> files = Collections.singletonList( absoluteFile );
try
{
List<MavenProject> projects = projectsSelector.selectProjects(
files, request );
+ if ( moduleProjectPomFile.isPresent()
Review Comment:
Are you sure you need to check for `moduleProjectPomFile.isPresent()`?
Because previously, the warning would be emitted if the expected POM file did
_not_ exist - in which case we now return `Optional.empty()`.
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -103,25 +118,20 @@ public List<MavenProject> collectProjects(
MavenExecutionRequest request ) throw
}
}
- private File getMultiModuleProjectPomFile( MavenExecutionRequest request )
+ private Optional<File> getMultiModuleProjectPomFile( MavenExecutionRequest
request ) // return Optional<File>
Review Comment:
```suggestion
private Optional<File> getMultiModuleProjectPomFile(
MavenExecutionRequest request )
```
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -64,23 +65,36 @@ public MultiModuleCollectionStrategy( ModelLocator
modelLocator, ProjectsSelecto
@Override
public List<MavenProject> collectProjects( MavenExecutionRequest request )
throws ProjectBuildingException
{
- File moduleProjectPomFile = getMultiModuleProjectPomFile( request );
- List<File> files = Collections.singletonList(
moduleProjectPomFile.getAbsoluteFile() );
+ Optional<File> moduleProjectPomFile = getMultiModuleProjectPomFile(
request );
+ File absoluteFile = moduleProjectPomFile.orElse( request.getPom()
).getAbsoluteFile();
+ List<File> files = Collections.singletonList( absoluteFile );
try
{
List<MavenProject> projects = projectsSelector.selectProjects(
files, request );
+ if ( moduleProjectPomFile.isPresent()
+ && projects.stream()
Review Comment:
For better understanding of the `if` statement, I think it would help if the
second part would be refactored to a separate boolean variable with a
meaningful name.
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -91,7 +105,8 @@ public List<MavenProject> collectProjects(
MavenExecutionRequest request ) throw
if ( fallThrough )
{
- LOGGER.debug( "Multi module project collection failed:{}"
+ LOGGER.debug(
Review Comment:
It seems to me that these lines are unchanged apart from some whitespace. If
that is the case, could you please revert the lines that aren't changed?
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -64,23 +65,36 @@ public MultiModuleCollectionStrategy( ModelLocator
modelLocator, ProjectsSelecto
@Override
public List<MavenProject> collectProjects( MavenExecutionRequest request )
throws ProjectBuildingException
{
- File moduleProjectPomFile = getMultiModuleProjectPomFile( request );
- List<File> files = Collections.singletonList(
moduleProjectPomFile.getAbsoluteFile() );
+ Optional<File> moduleProjectPomFile = getMultiModuleProjectPomFile(
request );
+ File absoluteFile = moduleProjectPomFile.orElse( request.getPom()
).getAbsoluteFile();
+ List<File> files = Collections.singletonList( absoluteFile );
try
{
List<MavenProject> projects = projectsSelector.selectProjects(
files, request );
+ if ( moduleProjectPomFile.isPresent()
+ && projects.stream()
+ .filter( MavenProject::hasParent )
+ .noneMatch( p -> p.getParent().getBasedir() == null ) )
+ {
+ LOGGER.info( "Maven detected that the requested POM file is
part of a multi-module project, "
+ + "but could not find a pom.xml file in the
multi-module root directory '{}'.",
+ request.getMultiModuleProjectDirectory() );
+ LOGGER.info( "The reactor is limited to all projects under: "
+ request.getPom().getParent() );
+ }
+
boolean isRequestedProjectCollected = isRequestedProjectCollected(
request, projects );
if ( isRequestedProjectCollected )
{
return projects;
}
else
{
- LOGGER.debug( "Multi module project collection failed:{}"
- + "Detected a POM file next to a .mvn directory in a
parent directory ({}). "
- + "Maven assumed that POM file to be the parent of the
requested project ({}), but it turned "
- + "out that it was not. Another project collection
strategy will be executed as result.",
- System.lineSeparator(),
moduleProjectPomFile.getAbsolutePath(),
+ LOGGER.debug(
Review Comment:
It seems to me that these lines are unchanged apart from some whitespace. If
that is the case, could you please revert the lines that aren't changed?
--
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]
