[ 
https://issues.apache.org/jira/browse/MNG-7625?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17647622#comment-17647622
 ] 

ASF GitHub Bot commented on MNG-7625:
-------------------------------------

gnodet commented on code in PR #909:
URL: https://github.com/apache/maven/pull/909#discussion_r1048744963


##########
maven-core/src/main/java/org/apache/maven/settings/SettingsUtils.java:
##########
@@ -51,6 +52,25 @@ public static Settings merge(Settings dominant, Settings 
recessive, String reces
         return new MavenSettingsMerger().merge(dominant, recessive, 
recessiveSourceLevel);
     }
 
+    /**
+     * @param dominant
+     * @param recessive
+     * @param recessiveSourceLevel
+     * @deprecated please use {@link #merge(Settings, Settings, String)}
+     */
+    @Deprecated
+    public static void merge(
+            org.apache.maven.settings.Settings dominant,
+            org.apache.maven.settings.Settings recessive,
+            String recessiveSourceLevel) {
+
+        if (dominant == null || recessive == null) {
+            return;
+        }
+
+        dominant.delegate = new SettingsMerger().merge(dominant.getDelegate(), 
recessive.getDelegate(), true, null);

Review Comment:
   > > The only non immutable bit in the m4 `Settings` is the `sourceLevel` 
which I also have no idea where/why/how/if it's used.
   > 
   > Exactly, and `sourceLevel` can be set once - it is a reason why I used 
`SettingsMerger`.
   
   Ah, I see.
   
   > m-invoker-p has a hack for it: 
https://github.com/apache/maven-invoker-plugin/blob/bfb75f9e52e93478dab710bb7243978c06f48d1a/src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java#L1580-L1618
   
   > but only works with Maven3 - 4 has session in delegate field
   
   Ok, I think that's not a real issue, right ? As exception is caught and 
forgot.  But this would allow to get rid of this code in the future.
   
   > We also can relax of settings `sourceLevel`.
   
   I would be in favour of getting rid of it in the v4 api if it's not used, 
and worse, actually worked around.
    
   > By the way we have double implementation `MavenSettingsMerger` and 
`SettingsMerger` IMHO we should use one.
   
   I wasn't sure if the results would be the same, given the first one is 
hand-crafted and the second one generated.
   If that's the case, the easier would be to deprecate `MavenSettingsMerger` 
and have it delegated to `SettingsMerger`.  The `MavenSettingsMerger` is also 
the one used by default in the `DefaultSettingsBuilder`, but we can try and see 
if the ITs raise any issues.
   
   Also, if the goal is to restore compatibility, maybe a cleaner way would be 
similar to https://github.com/apache/maven/pull/874, i.e. restore the methods 
as they were in m3, but delegating to a new v4 similar class.  Both PRs are 
conflicting but #874 also has additional value and should have been merged 
earlier imho.





> Restore compatibility with Maven 3 - SettingsUtils#merge
> --------------------------------------------------------
>
>                 Key: MNG-7625
>                 URL: https://issues.apache.org/jira/browse/MNG-7625
>             Project: Maven
>          Issue Type: Task
>    Affects Versions: 4.0.0-alpha-3
>            Reporter: Slawomir Jaranowski
>            Assignee: Slawomir Jaranowski
>            Priority: Major
>
> Maven 3 has method:
> {code:java}
> void SettingsUtils#merge(org.apache.maven.settings.Settings, 
> org.apache.maven.settings.Settings, java.lang.String)
> {code}
> It is used by {{maven-invoker-plugin}}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to