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