You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/08/05 13:42:39 UTC
[GitHub] [maven] MartinKanters opened a new pull request, #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
MartinKanters opened a new pull request, #783:
URL: https://github.com/apache/maven/pull/783
This PR ensures that when a multi module project is resumed halfway, the project progress counter is not reset.
[Related issue](https://issues.apache.org/jira/browse/MNG-7098).
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[MNG-XXX] SUMMARY`, where you replace `MNG-XXX`
and `SUMMARY` with the appropriate JIRA issue. Best practice is to use the JIRA issue
title in the pull request title and in the first line of the commit message.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [x] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
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] MartinKanters merged pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
MartinKanters merged PR #783:
URL: https://github.com/apache/maven/pull/783
--
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] Giovds commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
Giovds commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r970407944
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
It makes sense to me to keep using this method. Perhaps the comment could be removed to avoid confusion as it is not annotated with `@Deprecated` and it is used in multiple places. I agree that these kind of "deprecation messages" are quite useless indeed.
--
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] michael-o commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r972986997
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
I would at least raise a JIRA issue.
--
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] MartinKanters commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r966805017
##########
maven-embedder/src/test/java/org/apache/maven/cli/event/ExecutionEventLoggerTest.java:
##########
@@ -191,6 +194,80 @@ public void testProjectStartedNoPom()
inOrder.verify( logger ).info( "--------------------------------[ pom ]---------------------------------" );
}
+ @Test
+ void testMultiModuleProjectProgress()
+ {
+ // prepare
+ MavenProject project1 = generateMavenProject("Apache Maven Embedder 1");
+ MavenProject project2 = generateMavenProject("Apache Maven Embedder 2");
+ MavenProject project3 = generateMavenProject("Apache Maven Embedder 3");
+
+ MavenSession session = mock( MavenSession.class );
+ when( session.getProjects() ).thenReturn( ImmutableList.of( project1, project2, project3 ) );
+ when( session.getAllProjects() ).thenReturn( ImmutableList.of( project1, project2, project3 ) );
+
+ ExecutionEvent sessionStartedEvent = mock( ExecutionEvent.class );
+ when( sessionStartedEvent.getSession() ).thenReturn( session );
+ ExecutionEvent projectStartedEvent1 = mock( ExecutionEvent.class );
+ when( projectStartedEvent1.getProject() ).thenReturn( project1 );
+ ExecutionEvent projectStartedEvent2 = mock( ExecutionEvent.class );
+ when( projectStartedEvent2.getProject() ).thenReturn( project2 );
+ ExecutionEvent projectStartedEvent3 = mock( ExecutionEvent.class );
+ when( projectStartedEvent3.getProject() ).thenReturn( project3 );
+
+ // execute
+ executionEventLogger.sessionStarted( sessionStartedEvent );
+ executionEventLogger.projectStarted( projectStartedEvent1 );
+ executionEventLogger.projectStarted( projectStartedEvent2 );
+ executionEventLogger.projectStarted( projectStartedEvent3 );
+
+ // verify
+ InOrder inOrder = inOrder( logger );
+ inOrder.verify( logger ).info( "Building Apache Maven Embedder 1 3.5.4-SNAPSHOT [1/3]" );
Review Comment:
Great suggestion. I thought I would have to use ArgumentCaptors for this, but I realised (regex) matchers also work for methods under `verify`. Thanks!
--
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] MartinKanters commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r973038017
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
A JIRA for removing that method and adding the JIRA number into the TODO?
--
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] Giovds commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
Giovds commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r966412679
##########
maven-embedder/src/test/java/org/apache/maven/cli/event/ExecutionEventLoggerTest.java:
##########
@@ -191,6 +194,80 @@ public void testProjectStartedNoPom()
inOrder.verify( logger ).info( "--------------------------------[ pom ]---------------------------------" );
}
+ @Test
+ void testMultiModuleProjectProgress()
+ {
+ // prepare
+ MavenProject project1 = generateMavenProject("Apache Maven Embedder 1");
+ MavenProject project2 = generateMavenProject("Apache Maven Embedder 2");
+ MavenProject project3 = generateMavenProject("Apache Maven Embedder 3");
+
+ MavenSession session = mock( MavenSession.class );
+ when( session.getProjects() ).thenReturn( ImmutableList.of( project1, project2, project3 ) );
+ when( session.getAllProjects() ).thenReturn( ImmutableList.of( project1, project2, project3 ) );
+
+ ExecutionEvent sessionStartedEvent = mock( ExecutionEvent.class );
+ when( sessionStartedEvent.getSession() ).thenReturn( session );
+ ExecutionEvent projectStartedEvent1 = mock( ExecutionEvent.class );
+ when( projectStartedEvent1.getProject() ).thenReturn( project1 );
+ ExecutionEvent projectStartedEvent2 = mock( ExecutionEvent.class );
+ when( projectStartedEvent2.getProject() ).thenReturn( project2 );
+ ExecutionEvent projectStartedEvent3 = mock( ExecutionEvent.class );
+ when( projectStartedEvent3.getProject() ).thenReturn( project3 );
+
+ // execute
+ executionEventLogger.sessionStarted( sessionStartedEvent );
+ executionEventLogger.projectStarted( projectStartedEvent1 );
+ executionEventLogger.projectStarted( projectStartedEvent2 );
+ executionEventLogger.projectStarted( projectStartedEvent3 );
+
+ // verify
+ InOrder inOrder = inOrder( logger );
+ inOrder.verify( logger ).info( "Building Apache Maven Embedder 1 3.5.4-SNAPSHOT [1/3]" );
Review Comment:
It might be an idea to validate just the counter and the project name? This way the test cases will not fail if it's decided to change the amount of whitespaces, or when the wording of the sentence changes for example.
--
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] MartinKanters commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r966788232
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
Yeah that's a good point. It's been there since 2014, though and the documentation of the member field shows exactly what we need :)
```
/**
* The full set of projects before any potential constraining by --projects. Useful in the case where you want to
* build a smaller set of projects but perform other operations in the context of your reactor.
*/
```
I'm inclined to keep it, what do you think? :)
--
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] Giovds commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
Giovds commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r966396162
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
Perhaps it might be worth noting that the `MavenSession#getAllProjects()` [states](https://github.com/apache/maven/blob/3c487fa697c3ff29d6c79ab5cf6e834abfd143b3/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java#L273) `This is a provisional method and may be removed`. However long that may have been there :see_no_evil:.
--
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] michael-o commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r966979227
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
As hard as it sounds, I deprecation without even a slight note is quite useless...
--
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] MartinKanters commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r972950966
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
Makes sense, but I am not sure what the policy is on that. @michael-o Should I remove it as suggested, what do you think?
--
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] michael-o commented on a diff in pull request #783: [MNG-7098] Keep the project counter intact when resuming a multi-module project.
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #783:
URL: https://github.com/apache/maven/pull/783#discussion_r973131293
##########
maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java:
##########
@@ -143,7 +143,11 @@ public void sessionStarted( ExecutionEvent event )
project.getName(), chars( ' ', ( len > 0 ) ? len : 1 ), project.getPackaging() );
}
- totalProjects = projects.size();
+ final List<MavenProject> allProjects = event.getSession().getAllProjects();
Review Comment:
Yes
--
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