[GitHub] [maven] gnodet commented on a change in pull request #661: [MNG-7391] add execution strategy+runner required by Maven Build Cache

[GitBox <gi...@apache.org>]

gnodet commented on a change in pull request #661:
URL: https://github.com/apache/maven/pull/661#discussion_r801487087



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java
##########
@@ -45,7 +45,7 @@
 @Singleton
 public class DefaultLifecycles
 {
-    public static final String[] STANDARD_LIFECYCLES = { "default", "clean", "site", "wrapper" };
+    public static final String[] STANDARD_LIFECYCLES = { "clean", "default", "site", "wrapper" };

Review comment:
       Yes, the change is required.  
   
   The current code (without this PR) provides two different ordering for the lifecycles, one is given by `DefaultLifecycles#getLifeCycles()` which sorts the lifecycles according to the `STANDARD_LIFECYCLES` array.  The other one comes `DefaultLifecyclePluginAnalyzer#getOrderedLifecycles()` which sorts the lifecycles alphabetically.
   My analysis is that the first order isn't really used, while the second one is actually while computing the goals that will be invoked from the phases given on the command line.
   I thus changed the first order to actually return the same thing that the second one and not rely on alphabetical ordering which imho does not make sense semantically : the `clean` phase should be executed before the `default` and the `site` after the `default`, and the alphabetical order is not really relevant.
   
   If it's problematic, I'll revert that part and will provide another sort just for the use case.




-- 
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