You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/01/06 07:19:03 UTC
[GitHub] [maven] hboutemy opened a new pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
hboutemy opened a new pull request #651:
URL: https://github.com/apache/maven/pull/651
same as #650 / MNG-7263 but on per-packaging plugins bindings for default lifecycle
--
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
[GitHub] [maven] hboutemy commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r779733342
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -22,50 +22,40 @@
import java.util.Collections;
import java.util.HashMap;
-import javax.inject.Inject;
-import javax.inject.Named;
import javax.inject.Provider;
-import javax.inject.Singleton;
import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
import org.apache.maven.lifecycle.mapping.Lifecycle;
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
import org.apache.maven.lifecycle.mapping.LifecyclePhase;
-@Named( "pom" )
-@Singleton
-public final class PomLifecycleMappingProvider
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
implements Provider<LifecycleMapping>
{
- private final LifecycleMapping lifecycleMapping;
-
- @Inject
- public PomLifecycleMappingProvider()
- {
- HashMap<String, LifecyclePhase> lifecyclePhases = new HashMap<>();
- lifecyclePhases.put(
- "install",
- new LifecyclePhase( "org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install" )
- );
- lifecyclePhases.put(
- "deploy",
- new LifecyclePhase( "org.apache.maven.plugins:maven-deploy-plugin:3.0.0-M1:deploy" )
- );
-
- Lifecycle lifecycle = new Lifecycle();
- lifecycle.setId( "default" );
- lifecycle.setLifecyclePhases( Collections.unmodifiableMap( lifecyclePhases ) );
-
- this.lifecycleMapping = new DefaultLifecycleMapping(
- Collections.singletonList(
- lifecycle
- )
- );
- }
-
- @Override
- public LifecycleMapping get()
- {
- return lifecycleMapping;
- }
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
Review comment:
previously, everything was pure XML parsing: Tamasz switched to Provider code, but still using old `new LifecyclePhase( String )` that parses (and splits on comma if there are multiple bindings
so we're going step by step from Plexus XML to perfectly maintainable code (and maintainable code will match the documentation objective: code before this PR is just not readable for documentation, then is not maintainable either = what is even more important than documentation)
--
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
[GitHub] [maven] cstamas commented on pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #651:
URL: https://github.com/apache/maven/pull/651#issuecomment-1006477916
I'd propose following change (to both related PRs): https://github.com/apache/maven/pull/652
--
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
[GitHub] [maven] hboutemy commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r779733342
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -22,50 +22,40 @@
import java.util.Collections;
import java.util.HashMap;
-import javax.inject.Inject;
-import javax.inject.Named;
import javax.inject.Provider;
-import javax.inject.Singleton;
import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
import org.apache.maven.lifecycle.mapping.Lifecycle;
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
import org.apache.maven.lifecycle.mapping.LifecyclePhase;
-@Named( "pom" )
-@Singleton
-public final class PomLifecycleMappingProvider
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
implements Provider<LifecycleMapping>
{
- private final LifecycleMapping lifecycleMapping;
-
- @Inject
- public PomLifecycleMappingProvider()
- {
- HashMap<String, LifecyclePhase> lifecyclePhases = new HashMap<>();
- lifecyclePhases.put(
- "install",
- new LifecyclePhase( "org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install" )
- );
- lifecyclePhases.put(
- "deploy",
- new LifecyclePhase( "org.apache.maven.plugins:maven-deploy-plugin:3.0.0-M1:deploy" )
- );
-
- Lifecycle lifecycle = new Lifecycle();
- lifecycle.setId( "default" );
- lifecycle.setLifecyclePhases( Collections.unmodifiableMap( lifecyclePhases ) );
-
- this.lifecycleMapping = new DefaultLifecycleMapping(
- Collections.singletonList(
- lifecycle
- )
- );
- }
-
- @Override
- public LifecycleMapping get()
- {
- return lifecycleMapping;
- }
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
Review comment:
previously, everything was pure XML parsing: Tamasz switched to Provider code, but still using old `new LifecyclePhase( String )` that parses (and splits on comma if there are multiple bindings).
So we're going step by step from Plexus XML to perfectly maintainable code (and maintainable code will match the documentation objective: code before this PR is just not readable for documentation, then is not maintainable either = what is even more important than documentation)
--
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
[GitHub] [maven] hboutemy merged pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
hboutemy merged pull request #651:
URL: https://github.com/apache/maven/pull/651
--
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
[GitHub] [maven] mthmulders commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r779783453
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -22,50 +22,40 @@
import java.util.Collections;
import java.util.HashMap;
-import javax.inject.Inject;
-import javax.inject.Named;
import javax.inject.Provider;
-import javax.inject.Singleton;
import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
import org.apache.maven.lifecycle.mapping.Lifecycle;
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
import org.apache.maven.lifecycle.mapping.LifecyclePhase;
-@Named( "pom" )
-@Singleton
-public final class PomLifecycleMappingProvider
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
implements Provider<LifecycleMapping>
{
- private final LifecycleMapping lifecycleMapping;
-
- @Inject
- public PomLifecycleMappingProvider()
- {
- HashMap<String, LifecyclePhase> lifecyclePhases = new HashMap<>();
- lifecyclePhases.put(
- "install",
- new LifecyclePhase( "org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install" )
- );
- lifecyclePhases.put(
- "deploy",
- new LifecyclePhase( "org.apache.maven.plugins:maven-deploy-plugin:3.0.0-M1:deploy" )
- );
-
- Lifecycle lifecycle = new Lifecycle();
- lifecycle.setId( "default" );
- lifecycle.setLifecyclePhases( Collections.unmodifiableMap( lifecyclePhases ) );
-
- this.lifecycleMapping = new DefaultLifecycleMapping(
- Collections.singletonList(
- lifecycle
- )
- );
- }
-
- @Override
- public LifecycleMapping get()
- {
- return lifecycleMapping;
- }
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
Review comment:
Makes sense, one improvement at a time... Getting rid of the XML descriptors is a big win already!
--
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
[GitHub] [maven] hboutemy commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r779733342
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -22,50 +22,40 @@
import java.util.Collections;
import java.util.HashMap;
-import javax.inject.Inject;
-import javax.inject.Named;
import javax.inject.Provider;
-import javax.inject.Singleton;
import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
import org.apache.maven.lifecycle.mapping.Lifecycle;
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
import org.apache.maven.lifecycle.mapping.LifecyclePhase;
-@Named( "pom" )
-@Singleton
-public final class PomLifecycleMappingProvider
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
implements Provider<LifecycleMapping>
{
- private final LifecycleMapping lifecycleMapping;
-
- @Inject
- public PomLifecycleMappingProvider()
- {
- HashMap<String, LifecyclePhase> lifecyclePhases = new HashMap<>();
- lifecyclePhases.put(
- "install",
- new LifecyclePhase( "org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install" )
- );
- lifecyclePhases.put(
- "deploy",
- new LifecyclePhase( "org.apache.maven.plugins:maven-deploy-plugin:3.0.0-M1:deploy" )
- );
-
- Lifecycle lifecycle = new Lifecycle();
- lifecycle.setId( "default" );
- lifecycle.setLifecyclePhases( Collections.unmodifiableMap( lifecyclePhases ) );
-
- this.lifecycleMapping = new DefaultLifecycleMapping(
- Collections.singletonList(
- lifecycle
- )
- );
- }
-
- @Override
- public LifecycleMapping get()
- {
- return lifecycleMapping;
- }
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
Review comment:
previously, everything was pure XML parsing: Tamás switched to Provider code, but still using old `new LifecyclePhase( String )` that parses (and splits on comma if there are multiple bindings).
So we're going step by step from Plexus XML to perfectly maintainable code (and maintainable code will match the documentation objective: code before this PR is just not readable for documentation, then is not maintainable either = what is even more important than documentation)
--
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
[GitHub] [maven] cstamas commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r780107658
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.lifecycle.providers.packaging;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Collections;
+import java.util.HashMap;
+
+import javax.inject.Provider;
+
+import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
+import org.apache.maven.lifecycle.mapping.Lifecycle;
+import org.apache.maven.lifecycle.mapping.LifecycleMapping;
+import org.apache.maven.lifecycle.mapping.LifecyclePhase;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
+ implements Provider<LifecycleMapping>
+{
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
+ {
+ requireNonNull( pluginBindings );
+ final int len = pluginBindings.length;
+ if ( len < 1 || len % 2 != 0 )
Review comment:
do we allow "null" (empty) lifecycle? If so, remove the `len <. 1`. If not, make it `len < 2`
--
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
[GitHub] [maven] cstamas commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r780107658
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.lifecycle.providers.packaging;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Collections;
+import java.util.HashMap;
+
+import javax.inject.Provider;
+
+import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
+import org.apache.maven.lifecycle.mapping.Lifecycle;
+import org.apache.maven.lifecycle.mapping.LifecycleMapping;
+import org.apache.maven.lifecycle.mapping.LifecyclePhase;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
+ implements Provider<LifecycleMapping>
+{
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
+ {
+ requireNonNull( pluginBindings );
+ final int len = pluginBindings.length;
+ if ( len < 1 || len % 2 != 0 )
Review comment:
do we allow "null" (empty) lifecycle? If so, remove the `len < 1`. If not, make it `len < 2`
--
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
[GitHub] [maven] hboutemy commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r780672842
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.lifecycle.providers.packaging;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Collections;
+import java.util.HashMap;
+
+import javax.inject.Provider;
+
+import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
+import org.apache.maven.lifecycle.mapping.Lifecycle;
+import org.apache.maven.lifecycle.mapping.LifecycleMapping;
+import org.apache.maven.lifecycle.mapping.LifecyclePhase;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
+ implements Provider<LifecycleMapping>
+{
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
+ {
+ requireNonNull( pluginBindings );
+ final int len = pluginBindings.length;
+ if ( len < 1 || len % 2 != 0 )
Review comment:
clearly, 0 does not make sense
in reality, the smallest existing packaging bindings have 2 bindings = install + deploy
--
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
[GitHub] [maven] cstamas commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r779506851
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -22,50 +22,40 @@
import java.util.Collections;
import java.util.HashMap;
-import javax.inject.Inject;
-import javax.inject.Named;
import javax.inject.Provider;
-import javax.inject.Singleton;
import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
import org.apache.maven.lifecycle.mapping.Lifecycle;
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
import org.apache.maven.lifecycle.mapping.LifecyclePhase;
-@Named( "pom" )
-@Singleton
-public final class PomLifecycleMappingProvider
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
implements Provider<LifecycleMapping>
{
- private final LifecycleMapping lifecycleMapping;
-
- @Inject
- public PomLifecycleMappingProvider()
- {
- HashMap<String, LifecyclePhase> lifecyclePhases = new HashMap<>();
- lifecyclePhases.put(
- "install",
- new LifecyclePhase( "org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install" )
- );
- lifecyclePhases.put(
- "deploy",
- new LifecyclePhase( "org.apache.maven.plugins:maven-deploy-plugin:3.0.0-M1:deploy" )
- );
-
- Lifecycle lifecycle = new Lifecycle();
- lifecycle.setId( "default" );
- lifecycle.setLifecyclePhases( Collections.unmodifiableMap( lifecyclePhases ) );
-
- this.lifecycleMapping = new DefaultLifecycleMapping(
- Collections.singletonList(
- lifecycle
- )
- );
- }
-
- @Override
- public LifecycleMapping get()
- {
- return lifecycleMapping;
- }
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
Review comment:
Goal is "ease of documentation" AFAIU... as before, doco was simply pointing to XML (that was nicely formatted and totally "human readable"), I think Herve plans the same thing here, just have doco point at (trivially simple) class. So while I agree that abstraction seems not the best, keep in mind the reason of this change of his (if am not mistaken). And I agree, Java11 or Guava ImmutableMaps would really help here to build a Map in "readable" way.
--
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
[GitHub] [maven] mthmulders commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r779474475
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -22,50 +22,40 @@
import java.util.Collections;
import java.util.HashMap;
-import javax.inject.Inject;
-import javax.inject.Named;
import javax.inject.Provider;
-import javax.inject.Singleton;
import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
import org.apache.maven.lifecycle.mapping.Lifecycle;
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
import org.apache.maven.lifecycle.mapping.LifecyclePhase;
-@Named( "pom" )
-@Singleton
-public final class PomLifecycleMappingProvider
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
implements Provider<LifecycleMapping>
{
- private final LifecycleMapping lifecycleMapping;
-
- @Inject
- public PomLifecycleMappingProvider()
- {
- HashMap<String, LifecyclePhase> lifecyclePhases = new HashMap<>();
- lifecyclePhases.put(
- "install",
- new LifecyclePhase( "org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install" )
- );
- lifecyclePhases.put(
- "deploy",
- new LifecyclePhase( "org.apache.maven.plugins:maven-deploy-plugin:3.0.0-M1:deploy" )
- );
-
- Lifecycle lifecycle = new Lifecycle();
- lifecycle.setId( "default" );
- lifecycle.setLifecyclePhases( Collections.unmodifiableMap( lifecyclePhases ) );
-
- this.lifecycleMapping = new DefaultLifecycleMapping(
- Collections.singletonList(
- lifecycle
- )
- );
- }
-
- @Override
- public LifecycleMapping get()
- {
- return lifecycleMapping;
- }
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
Review comment:
If I'm not mistaken, all plugin bindings are a one-to-one mapping of phase -> plugin (see subclasses). Why not change this parameter to a `Map<String, String>`, and have subclasses pass the mapping as a `Map<String, String>`? It feels as if a `String[]` (where each consecutive two elements form a "pair") is the wrong abstraction in this 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
[GitHub] [maven] cstamas commented on a change in pull request #651: [MNG-7264] refactor packaging bindings providers to ease maintenance
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #651:
URL: https://github.com/apache/maven/pull/651#discussion_r780107658
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.lifecycle.providers.packaging;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Collections;
+import java.util.HashMap;
+
+import javax.inject.Provider;
+
+import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
+import org.apache.maven.lifecycle.mapping.Lifecycle;
+import org.apache.maven.lifecycle.mapping.LifecycleMapping;
+import org.apache.maven.lifecycle.mapping.LifecyclePhase;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
+ implements Provider<LifecycleMapping>
+{
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
+ {
+ requireNonNull( pluginBindings );
+ final int len = pluginBindings.length;
+ if ( len < 1 || len % 2 != 0 )
Review comment:
do we allow "null" (empty) lifecycle? If so, remove the `len <. 1`. If not, make it `len < 2`
##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/providers/packaging/AbstractLifecycleMappingProvider.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.lifecycle.providers.packaging;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Collections;
+import java.util.HashMap;
+
+import javax.inject.Provider;
+
+import org.apache.maven.lifecycle.mapping.DefaultLifecycleMapping;
+import org.apache.maven.lifecycle.mapping.Lifecycle;
+import org.apache.maven.lifecycle.mapping.LifecycleMapping;
+import org.apache.maven.lifecycle.mapping.LifecyclePhase;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Base lifecycle mapping provider, ie per-packaging plugin bindings for {@code default} lifecycle.
+ */
+public abstract class AbstractLifecycleMappingProvider
+ implements Provider<LifecycleMapping>
+{
+ private final LifecycleMapping lifecycleMapping;
+
+ protected AbstractLifecycleMappingProvider( String[] pluginBindings )
+ {
+ requireNonNull( pluginBindings );
+ final int len = pluginBindings.length;
+ if ( len < 1 || len % 2 != 0 )
Review comment:
do we allow "null" (empty) lifecycle? If so, remove the `len < 1`. If not, make it `len < 2`
--
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