[GitHub] [maven] MartinKanters commented on pull request #444: [MNG-7095] Fix resume for parallel builds

2021-02-13 Thread GitBox


MartinKanters commented on pull request #444:
URL: https://github.com/apache/maven/pull/444#issuecomment-778597830


   Thanks for your contribution @gnodet ! 



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] MartinKanters commented on pull request #444: [MNG-7095] Fix resume for parallel builds

2021-02-12 Thread GitBox


MartinKanters commented on pull request #444:
URL: https://github.com/apache/maven/pull/444#issuecomment-778436709


   Cool, your ITs are looking good right now, so the PR can be merged in my 
opinion. @mthmulders do you want to take another look? Otherwise I can merge 
somewhere this weekend.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] MartinKanters commented on pull request #444: [MNG-7095] Fix resume for parallel builds

2021-02-12 Thread GitBox


MartinKanters commented on pull request #444:
URL: https://github.com/apache/maven/pull/444#issuecomment-778412923


   > > > @mthmulders @MartinKanters quick question.
   > > > I'm working on improving the integration test and have the following 
scenario: a parent, 3 modules named `A`, `B`, `C` with `C` depending on `B` 
(that's the `mng-5760/parent-dependent` test project).
   > > > I run the `test` goal on the project with `-T2` and with `A` and `C` 
failing, which means `B` succeeds. When resuming, `C` fails because it can not 
find the artifact `B`. That's because the goal run previously was `test` and 
not `install` so the artifact is not in the local repository. However, 
shouldn't maven be _aware_ that the project is in the local reactor somehow so 
that the build can succeed ?
   > > 
   > > 
   > > @gnodet I would also expect that to succeed. Although module B hadn't 
been packaged in the earlier build, it does have a folder with compiled 
classes, and that should be enough. Can you isolate that behaviour into a 
separate test project so I can have a look at that later?
   > 
   > I've pushed 
[apache/maven-integration-testing#98](https://github.com/apache/maven-integration-testing/pull/98)
 which contains 2 integration tests for the parallel build with the resume flag.
   > 
   > I'll create another PR with an integration test for the other problem.
   
   You've encountered the bug that MNG-4660 has fixed, but is not released in a 
main Maven release yet. With the latest Maven snapshot build you should not 
have to use install anymore.
   
   I did review your other ITs as well, they look good (apart from some small 
points).



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org