gnodet commented on code in PR #12327:
URL: https://github.com/apache/maven/pull/12327#discussion_r3441833242
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PluginDependenciesResolver.java:
##########
@@ -61,12 +62,34 @@ Artifact resolve(Plugin plugin, List<RemoteRepository>
repositories, RepositoryS
* @param session The repository session to use for resolving the plugin
artifacts, must not be {@code null}.
* @return The dependency tree denoting the resolved plugin class path,
never {@code null}.
* @throws PluginResolutionException If any dependency could not be
resolved.
+ * @deprecated This method should be avoided, as it needs manual
flattening, instead to let Resolver do it. Use {@link
#resolveAndFlatten(Plugin, Artifact, DependencyFilter, List,
RepositorySystemSession)} instead.
Review Comment:
Nit: `"instead to let Resolver do it"` reads awkwardly. Suggestion:
```suggestion
* @deprecated This method should be avoided, as it requires manual
flattening; use {@link #resolveAndFlatten(Plugin, Artifact, DependencyFilter,
List, RepositorySystemSession)} instead to let Resolver handle it.
```
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PluginDependenciesResolver.java:
##########
@@ -61,12 +62,34 @@ Artifact resolve(Plugin plugin, List<RemoteRepository>
repositories, RepositoryS
* @param session The repository session to use for resolving the plugin
artifacts, must not be {@code null}.
* @return The dependency tree denoting the resolved plugin class path,
never {@code null}.
* @throws PluginResolutionException If any dependency could not be
resolved.
+ * @deprecated This method should be avoided, as it needs manual
flattening, instead to let Resolver do it. Use {@link
#resolveAndFlatten(Plugin, Artifact, DependencyFilter, List,
RepositorySystemSession)} instead.
*/
+ @Deprecated
DependencyNode resolve(
Plugin plugin,
Artifact pluginArtifact,
DependencyFilter dependencyFilter,
List<RemoteRepository> repositories,
RepositorySystemSession session)
throws PluginResolutionException;
+
+ /**
+ * Resolves the runtime dependencies of the specified plugin.
+ *
+ * @param plugin The plugin for which to resolve the dependencies, must
not be {@code null}.
+ * @param pluginArtifact The plugin's main artifact, may be {@code null}.
+ * @param dependencyFilter A filter to exclude artifacts from resolution
(but not collection), may be {@code null}.
+ * @param repositories The plugin repositories to use for resolving the
plugin artifacts, must not be {@code null}.
+ * @param session The repository session to use for resolving the plugin
artifacts, must not be {@code null}.
+ * @return The dependency resolution result having the resolved plugin
class path but also the tree, never {@code null}.
+ * @throws PluginResolutionException If any dependency could not be
resolved.
+ * @since 3.10.0
+ */
+ DependencyResult resolveAndFlatten(
Review Comment:
Question: on master (Maven 4), the equivalent method is named
`resolvePlugin()` and `resolveCoreExtension()` already returns
`DependencyResult` directly. The `AndFlatten` suffix is arguably more
descriptive, but it introduces a naming divergence from Maven 4 — which is the
opposite of the PR's stated alignment goal. Was this intentional, or would
`resolvePlugin` / keeping `resolveCoreExtension` (with a changed return type)
be preferred for consistency?
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -161,20 +163,46 @@ public DependencyNode resolveCoreExtension(
List<RemoteRepository> repositories,
RepositorySystemSession session)
throws PluginResolutionException {
+ return resolveInternal(plugin, null /* pluginArtifact */,
dependencyFilter, repositories, session)
+ .getRoot();
+ }
+
+ /**
+ * @since 3.10.0
+ */
+ public DependencyResult resolveCoreExtensionAndFlatten(
Review Comment:
This method is `public` but not declared on the `PluginDependenciesResolver`
interface. `BootstrapCoreExtensionManager` calls it, so it depends on the
concrete type. The old `resolveCoreExtension` had the same pattern, but since
the interface is already being touched to add `resolveAndFlatten`, would it
make sense to also promote this to the interface?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]