You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/03/07 16:26:21 UTC
[GitHub] [maven] cstamas opened a new pull request, #1040: [MNG-7720] Wrong build order of forked projects
cstamas opened a new pull request, #1040:
URL: https://github.com/apache/maven/pull/1040
Forward port of fix.
---
https://issues.apache.org/jira/browse/MNG-7720
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on a diff in pull request #1040: [MNG-7720] Wrong build order of forked projects
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1040:
URL: https://github.com/apache/maven/pull/1040#discussion_r1128346697
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java:
##########
@@ -90,7 +90,11 @@ public LifecycleDependencyResolver(
public static List<MavenProject> getProjects(MavenProject project, MavenSession session, boolean aggregator) {
if (aggregator && project.getCollectedProjects() != null) {
Review Comment:
I think it can be only on "top level" null (ie. some goal invocation not project being built), and once top level is not null, no child will have it null either... this is just a wild guess.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slawekjaranowski commented on a diff in pull request #1040: [MNG-7720] Wrong build order of forked projects
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1040:
URL: https://github.com/apache/maven/pull/1040#discussion_r1128339820
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java:
##########
@@ -90,7 +90,11 @@ public LifecycleDependencyResolver(
public static List<MavenProject> getProjects(MavenProject project, MavenSession session, boolean aggregator) {
if (aggregator && project.getCollectedProjects() != null) {
Review Comment:
Here we have check `project.getCollectedProjects() != null`
but in method `getProjectAndSubModules` we list project `recursively` and we don't check if subproject has `getCollectedProjects() != null` - it is ok? line 102/106
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on a diff in pull request #1040: [MNG-7720] Wrong build order of forked projects
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1040:
URL: https://github.com/apache/maven/pull/1040#discussion_r1128379457
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java:
##########
@@ -90,7 +90,11 @@ public LifecycleDependencyResolver(
public static List<MavenProject> getProjects(MavenProject project, MavenSession session, boolean aggregator) {
if (aggregator && project.getCollectedProjects() != null) {
Review Comment:
It seems this is really "just implementation details" (that when top level project collectedProjects is not null, then there is no project instance that would have collectedProjects null). So added nullcheck just to be on safe side.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas merged pull request #1040: [MNG-7720] Wrong build order of forked projects
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #1040:
URL: https://github.com/apache/maven/pull/1040
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org