You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Olivier Lamy <ol...@apache.org> on 2013/12/16 11:27:28 UTC

Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

On 16 December 2013 15:11,  <if...@apache.org> wrote:
> Updated Branches:
>   refs/heads/master 07fecbd9e -> b99658c94
>
>
> MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener
>
> Signed-off-by: Igor Fedorenko <if...@apache.org>
>
>
> Project: http://git-wip-us.apache.org/repos/asf/maven/repo
> Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/bc84e6c7
> Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/bc84e6c7
> Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/bc84e6c7
>
> Branch: refs/heads/master
> Commit: bc84e6c7d307f119d82d5ae8082cc5e4f7eb7996
> Parents: 07fecbd
> Author: Igor Fedorenko <if...@apache.org>
> Authored: Sat Dec 14 19:43:42 2013 -0500
> Committer: Igor Fedorenko <if...@apache.org>
> Committed: Sat Dec 14 19:43:42 2013 -0500
>
> ----------------------------------------------------------------------
>  .../maven/execution/MojoExecutionListener.java  |  46 +++++++
>  .../execution/ProjectExecutionListener.java     |  49 +++++++
>  .../execution/scope/MojoExecutionListener.java  |  48 -------
>  .../scope/WeakMojoExecutionListener.java        |  50 ++++++++
>  .../scope/internal/MojoExecutionScope.java      |  36 ++++--
>  .../CompoundProjectExecutionListener.java       |  76 +++++++++++
>  .../internal/LifecycleModuleBuilder.java        |  34 ++++-
>  .../plugin/CompoundMojoExecutionListener.java   |  67 ++++++++++
>  .../maven/plugin/DefaultBuildPluginManager.java |  33 ++++-
>  .../DelegatingMojoExecutionListener.java        |  61 +++++++++
>  .../DelegatingProjectExecutionListener.java     |  66 ++++++++++
>  .../maven/lifecycle/LifecycleExecutorTest.java  | 127 +++++++++++++++++++
>  12 files changed, 626 insertions(+), 67 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/execution/MojoExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/execution/MojoExecutionListener.java b/maven-core/src/main/java/org/apache/maven/execution/MojoExecutionListener.java
> new file mode 100644
> index 0000000..6b824eb
> --- /dev/null
> +++ b/maven-core/src/main/java/org/apache/maven/execution/MojoExecutionListener.java
> @@ -0,0 +1,46 @@
> +package org.apache.maven.execution;
> +
> +/*
> + * 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 org.apache.maven.execution.scope.WeakMojoExecutionListener;
> +import org.apache.maven.plugin.Mojo;
> +import org.apache.maven.plugin.MojoExecution;
> +import org.apache.maven.plugin.MojoExecutionException;
> +import org.apache.maven.project.MavenProject;
> +
> +/**
> + * Extension point that allows build extensions observe and possibly veto mojo executions.
> + *
> + * @see WeakMojoExecutionListener
> + * @since 3.1.2
> + * @provisional This interface is part of work in progress and can be changed or removed without notice.
> + */
> +public interface MojoExecutionListener
> +{
> +    public void beforeMojoExecution( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo )
> +        throws MojoExecutionException;
> +
> +    public void afterMojoExecutionSuccess( MavenSession session, MavenProject project, MojoExecution execution,
> +                                           Mojo mojo )
> +        throws MojoExecutionException;
> +
> +    public void afterExecutionFailure( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo,
> +                                       Throwable cause );
> +}

I wonder if it will be easier for future enhancement to use a bean
with fields for those objects.
MojoExecutionListenerEvent with getMavenSession() etc...

Maybe will be simpler to add getter to this bean than changing the
signature of the interface.
?


>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/execution/ProjectExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/execution/ProjectExecutionListener.java b/maven-core/src/main/java/org/apache/maven/execution/ProjectExecutionListener.java
> new file mode 100644
> index 0000000..d3d9739
> --- /dev/null
> +++ b/maven-core/src/main/java/org/apache/maven/execution/ProjectExecutionListener.java
> @@ -0,0 +1,49 @@
> +package org.apache.maven.execution;
> +
> +/*
> + * 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.List;
> +
> +import org.apache.maven.lifecycle.LifecycleExecutionException;
> +import org.apache.maven.plugin.MojoExecution;
> +import org.apache.maven.project.MavenProject;
> +
> +/**
> + * Extension point that allows build extensions observe and possibly veto project build execution.
> + *
> + * @see ExecutionListener
> + * @see MojoExecutionListener
> + * @since 3.1.2
> + * @provisional This interface is part of work in progress and can be changed or removed without notice.
> + */
> +public interface ProjectExecutionListener
> +{
> +    public void beforeProjectExecution( MavenSession session, MavenProject project )
> +        throws LifecycleExecutionException;
> +
> +    public void beforeProjectLifecycleExecution( MavenSession session, MavenProject project,
> +                                                 List<MojoExecution> executionPlan )
> +        throws LifecycleExecutionException;
> +
> +    public void afterProjectExecutionSuccess( MavenSession session, MavenProject project )
> +        throws LifecycleExecutionException;
> +
> +    public void afterProjectExecutionFailure( MavenSession session, MavenProject project, Throwable cause );
> +}
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/execution/scope/MojoExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/execution/scope/MojoExecutionListener.java b/maven-core/src/main/java/org/apache/maven/execution/scope/MojoExecutionListener.java
> deleted file mode 100644
> index 7a9c84b..0000000
> --- a/maven-core/src/main/java/org/apache/maven/execution/scope/MojoExecutionListener.java
> +++ /dev/null
> @@ -1,48 +0,0 @@
> -package org.apache.maven.execution.scope;
> -
> -/*
> - * 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 org.apache.maven.plugin.MojoExecutionException;
> -
> -/**
> - * Helper interface that allows mojo execution scoped components to be notified before execution start and after
> - * execution completion. The main purpose of this interface is to allow mojo execution scoped components perform setup
> - * before and cleanup after mojo execution.
> - * <p>
> - * TODO decide if Mojo should be passed as parameter of callback methods
> - *
> - * @author igor
> - * @since 3.1.2
> - * @provisional This interface is part of work in progress and can be changed or removed without notice.
> - */
> -public interface MojoExecutionListener
> -{
> -    // TODO decide if this is needed
> -    // public void beforeExecution() throws MojoExecutionException;
> -
> -    public void afterMojoExecutionSuccess()
> -        throws MojoExecutionException;
> -
> -    // TODO decide if this is needed.
> -    // public void afterExecutionFailure(Throwable t) throws MojoExecutionException;
> -
> -    public void afterMojoExecutionAlways()
> -        throws MojoExecutionException;
> -}
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/execution/scope/WeakMojoExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/execution/scope/WeakMojoExecutionListener.java b/maven-core/src/main/java/org/apache/maven/execution/scope/WeakMojoExecutionListener.java
> new file mode 100644
> index 0000000..5537106
> --- /dev/null
> +++ b/maven-core/src/main/java/org/apache/maven/execution/scope/WeakMojoExecutionListener.java
> @@ -0,0 +1,50 @@
> +package org.apache.maven.execution.scope;
> +
> +/*
> + * 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 org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.MojoExecutionListener;
> +import org.apache.maven.plugin.Mojo;
> +import org.apache.maven.plugin.MojoExecution;
> +import org.apache.maven.plugin.MojoExecutionException;
> +import org.apache.maven.project.MavenProject;
> +
> +/**
> + * Extension point that allows build extensions observe and possibly veto mojo executions.
> + * <p>
> + * Unlike {@link MojoExecutionListener}, this extension point does not trigger instantiation of the component, hence
> + * "weak" class name prefix. Only applies to mojo execution scoped components.
> + *
> + * @see MojoExecutionListener
> + * @since 3.1.2
> + * @provisional This interface is part of work in progress and can be changed or removed without notice.
> + */
> +public interface WeakMojoExecutionListener
> +{
> +    public void beforeMojoExecution( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo )
> +        throws MojoExecutionException;
> +
> +    public void afterMojoExecutionSuccess( MavenSession session, MavenProject project, MojoExecution execution,
> +                                           Mojo mojo )
> +        throws MojoExecutionException;
> +
> +    public void afterExecutionFailure( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo,
> +                                       Throwable cause );
> +}
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/execution/scope/internal/MojoExecutionScope.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/execution/scope/internal/MojoExecutionScope.java b/maven-core/src/main/java/org/apache/maven/execution/scope/internal/MojoExecutionScope.java
> index ce169e1..80badfc 100644
> --- a/maven-core/src/main/java/org/apache/maven/execution/scope/internal/MojoExecutionScope.java
> +++ b/maven-core/src/main/java/org/apache/maven/execution/scope/internal/MojoExecutionScope.java
> @@ -26,8 +26,10 @@ import javax.inject.Named;
>  import javax.inject.Singleton;
>
>  import org.apache.maven.execution.MavenSession;
> -import org.apache.maven.execution.scope.MojoExecutionListener;
> +import org.apache.maven.execution.MojoExecutionListener;
>  import org.apache.maven.execution.scope.MojoExecutionScoped;
> +import org.apache.maven.execution.scope.WeakMojoExecutionListener;
> +import org.apache.maven.plugin.Mojo;
>  import org.apache.maven.plugin.MojoExecution;
>  import org.apache.maven.plugin.MojoExecutionException;
>  import org.apache.maven.project.MavenProject;
> @@ -46,7 +48,7 @@ import com.google.inject.util.Providers;
>  @Named
>  @Singleton
>  public class MojoExecutionScope
> -    implements Scope
> +    implements Scope, MojoExecutionListener
>  {
>      private static final Provider<Object> SEEDED_KEY_PROVIDER = new Provider<Object>()
>      {
> @@ -138,7 +140,7 @@ public class MojoExecutionScope
>                  }
>
>                  T provided = (T) state.provided.get( key );
> -                if ( provided == null )
> +                if ( provided == null && unscoped != null )
>                  {
>                      provided = unscoped.get();
>                      state.provided.put( key, provided );
> @@ -174,28 +176,40 @@ public class MojoExecutionScope
>          };
>      }
>
> -    public void afterExecutionSuccess()
> +    public void beforeMojoExecution( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo )
>          throws MojoExecutionException
>      {
>          for ( Object provided : getScopeState().provided.values() )
>          {
> -            if ( provided instanceof MojoExecutionListener )
> +            if ( provided instanceof WeakMojoExecutionListener )
>              {
> -                ( (MojoExecutionListener) provided ).afterMojoExecutionSuccess();
> -                // TODO maybe deal with multiple MojoExecutionExceptions
> +                ( (WeakMojoExecutionListener) provided ).beforeMojoExecution( session, project, execution, mojo );
>              }
>          }
>      }
>
> -    public void afterExecutionAlways()
> +    public void afterMojoExecutionSuccess( MavenSession session, MavenProject project, MojoExecution execution,
> +                                           Mojo mojo )
>          throws MojoExecutionException
>      {
>          for ( Object provided : getScopeState().provided.values() )
>          {
> -            if ( provided instanceof MojoExecutionListener )
> +            if ( provided instanceof WeakMojoExecutionListener )
>              {
> -                ( (MojoExecutionListener) provided ).afterMojoExecutionAlways();
> -                // TODO maybe deal with multiple MojoExecutionExceptions
> +                ( (WeakMojoExecutionListener) provided ).afterMojoExecutionSuccess( session, project, execution, mojo );
> +            }
> +        }
> +    }
> +
> +    public void afterExecutionFailure( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo,
> +                                       Throwable cause )
> +    {
> +        for ( Object provided : getScopeState().provided.values() )
> +        {
> +            if ( provided instanceof WeakMojoExecutionListener )
> +            {
> +                ( (WeakMojoExecutionListener) provided ).afterExecutionFailure( session, project, execution, mojo,
> +                                                                                cause );
>              }
>          }
>      }
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/lifecycle/internal/CompoundProjectExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/CompoundProjectExecutionListener.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/CompoundProjectExecutionListener.java
> new file mode 100644
> index 0000000..8bdd210
> --- /dev/null
> +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/CompoundProjectExecutionListener.java
> @@ -0,0 +1,76 @@
> +package org.apache.maven.lifecycle.internal;
> +
> +/*
> + * 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.Collection;
> +import java.util.List;
> +
> +import org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.ProjectExecutionListener;
> +import org.apache.maven.lifecycle.LifecycleExecutionException;
> +import org.apache.maven.plugin.MojoExecution;
> +import org.apache.maven.project.MavenProject;
> +
> +class CompoundProjectExecutionListener
> +    implements ProjectExecutionListener
> +{
> +    private final Collection<ProjectExecutionListener> listeners;
> +
> +    public CompoundProjectExecutionListener( Collection<ProjectExecutionListener> listeners )
> +    {
> +        this.listeners = listeners; // NB this is live injected collection
> +    }
> +
> +    public void beforeProjectExecution( MavenSession session, MavenProject project )
> +        throws LifecycleExecutionException
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.beforeProjectExecution( session, project );
> +        }
> +    }
> +
> +    public void beforeProjectLifecycleExecution( MavenSession session, MavenProject project,
> +                                                 List<MojoExecution> executionPlan )
> +        throws LifecycleExecutionException
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.beforeProjectLifecycleExecution( session, project, executionPlan );
> +        }
> +    }
> +
> +    public void afterProjectExecutionSuccess( MavenSession session, MavenProject project )
> +        throws LifecycleExecutionException
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.afterProjectExecutionSuccess( session, project );
> +        }
> +    }
> +
> +    public void afterProjectExecutionFailure( MavenSession session, MavenProject project, Throwable cause )
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.afterProjectExecutionFailure( session, project, cause );
> +        }
> +    }
> +}
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java
> index c8a3cf4..95f1c19 100644
> --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java
> +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java
> @@ -19,20 +19,23 @@ package org.apache.maven.lifecycle.internal;
>   * under the License.
>   */
>
> +import java.util.HashSet;
> +import java.util.List;
> +
>  import org.apache.maven.artifact.Artifact;
>  import org.apache.maven.execution.BuildSuccess;
>  import org.apache.maven.execution.ExecutionEvent;
>  import org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.ProjectExecutionListener;
>  import org.apache.maven.lifecycle.MavenExecutionPlan;
> +import org.apache.maven.plugin.MojoExecution;
>  import org.apache.maven.project.MavenProject;
>  import org.codehaus.plexus.component.annotations.Component;
>  import org.codehaus.plexus.component.annotations.Requirement;
>
> -import java.util.HashSet;
> -
>  /**
>   * Builds one or more lifecycles for a full module
> - *
> + *
>   * @since 3.0
>   * @author Benjamin Bentmann
>   * @author Jason van Zyl
> @@ -53,6 +56,18 @@ public class LifecycleModuleBuilder
>      @Requirement
>      private ExecutionEventCatapult eventCatapult;
>
> +    private ProjectExecutionListener projectExecutionListener;
> +
> +    // this tricks plexus-component-metadata generate required metadata
> +    @Requirement
> +    private List<ProjectExecutionListener> projectExecutionListeners;
> +
> +    public void setProjectExecutionListeners( final List<ProjectExecutionListener> listeners )
> +    {
> +        this.projectExecutionListeners = listeners;
> +        this.projectExecutionListener = new CompoundProjectExecutionListener( listeners );
> +    }
> +
>      public void buildProject( MavenSession session, ReactorContext reactorContext, MavenProject currentProject,
>                                TaskSegment taskSegment )
>      {
> @@ -75,16 +90,23 @@ public class LifecycleModuleBuilder
>                  return;
>              }
>
> +            BuilderCommon.attachToThread( currentProject );
> +
> +            projectExecutionListener.beforeProjectExecution( rootSession, currentProject );
> +
>              eventCatapult.fire( ExecutionEvent.Type.ProjectStarted, session, null );
>
> -            BuilderCommon.attachToThread( currentProject );
>              MavenExecutionPlan executionPlan =
>                  builderCommon.resolveBuildPlan( session, currentProject, taskSegment, new HashSet<Artifact>() );
> +            List<MojoExecution> mojoExecutions = executionPlan.getMojoExecutions();
>
> -            mojoExecutor.execute( session, executionPlan.getMojoExecutions(), reactorContext.getProjectIndex() );
> +            projectExecutionListener.beforeProjectLifecycleExecution( rootSession, currentProject, mojoExecutions );
> +            mojoExecutor.execute( session, mojoExecutions, reactorContext.getProjectIndex() );
>
>              long buildEndTime = System.currentTimeMillis();
>
> +            projectExecutionListener.afterProjectExecutionSuccess( rootSession, currentProject );
> +
>              reactorContext.getResult().addBuildSummary(
>                  new BuildSuccess( currentProject, buildEndTime - buildStartTime ) );
>
> @@ -93,6 +115,8 @@ public class LifecycleModuleBuilder
>          catch ( Exception e )
>          {
>              builderCommon.handleBuildError( reactorContext, rootSession, session, currentProject, e, buildStartTime );
> +
> +            projectExecutionListener.afterProjectExecutionFailure( session, currentProject, e );
>          }
>          finally
>          {
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/plugin/CompoundMojoExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/plugin/CompoundMojoExecutionListener.java b/maven-core/src/main/java/org/apache/maven/plugin/CompoundMojoExecutionListener.java
> new file mode 100644
> index 0000000..7b5ff66
> --- /dev/null
> +++ b/maven-core/src/main/java/org/apache/maven/plugin/CompoundMojoExecutionListener.java
> @@ -0,0 +1,67 @@
> +package org.apache.maven.plugin;
> +
> +/*
> + * 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.Collection;
> +
> +import org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.MojoExecutionListener;
> +import org.apache.maven.project.MavenProject;
> +
> +class CompoundMojoExecutionListener
> +    implements MojoExecutionListener
> +{
> +
> +    private final Collection<MojoExecutionListener> listeners;
> +
> +    public CompoundMojoExecutionListener( Collection<MojoExecutionListener> listeners )
> +    {
> +        this.listeners = listeners; // NB this is live injected collection
> +    }
> +
> +    public void beforeMojoExecution( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo )
> +        throws MojoExecutionException
> +    {
> +        for ( MojoExecutionListener listener : listeners )
> +        {
> +            listener.beforeMojoExecution( session, project, execution, mojo );
> +        }
> +    }
> +
> +    public void afterMojoExecutionSuccess( MavenSession session, MavenProject project, MojoExecution execution,
> +                                           Mojo mojo )
> +        throws MojoExecutionException
> +    {
> +        for ( MojoExecutionListener listener : listeners )
> +        {
> +            listener.afterMojoExecutionSuccess( session, project, execution, mojo );
> +        }
> +    }
> +
> +    public void afterExecutionFailure( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo,
> +                                       Throwable cause )
> +    {
> +        for ( MojoExecutionListener listener : listeners )
> +        {
> +            listener.afterExecutionFailure( session, project, execution, mojo, cause );
> +        }
> +    }
> +
> +}
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/main/java/org/apache/maven/plugin/DefaultBuildPluginManager.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/main/java/org/apache/maven/plugin/DefaultBuildPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/DefaultBuildPluginManager.java
> index 949a228..5a10053 100644
> --- a/maven-core/src/main/java/org/apache/maven/plugin/DefaultBuildPluginManager.java
> +++ b/maven-core/src/main/java/org/apache/maven/plugin/DefaultBuildPluginManager.java
> @@ -24,6 +24,7 @@ import java.io.PrintStream;
>  import java.util.List;
>
>  import org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.MojoExecutionListener;
>  import org.apache.maven.execution.scope.internal.MojoExecutionScope;
>  import org.apache.maven.model.Plugin;
>  import org.apache.maven.plugin.descriptor.MojoDescriptor;
> @@ -52,6 +53,18 @@ public class DefaultBuildPluginManager
>      @Requirement
>      private MojoExecutionScope scope;
>
> +    private MojoExecutionListener mojoExecutionListener;
> +
> +    // this tricks plexus-component-metadata generate required metadata
> +    @Requirement( role = MojoExecutionListener.class )
> +    private List<MojoExecutionListener> mojoExecutionListeners;
> +
> +    public void setMojoExecutionListeners( final List<MojoExecutionListener> listeners )
> +    {
> +        this.mojoExecutionListeners = listeners;
> +        this.mojoExecutionListener = new CompoundMojoExecutionListener( listeners );
> +    }
> +
>      /**
>       * @param plugin
>       * @param repositories
> @@ -112,9 +125,11 @@ public class DefaultBuildPluginManager
>              // MavenProjectHelper.attachArtifact(..).
>              try
>              {
> +                mojoExecutionListener.beforeMojoExecution( oldSession, project, mojoExecution, mojo );
> +
>                  mojo.execute();
>
> -                scope.afterExecutionSuccess();
> +                mojoExecutionListener.afterMojoExecutionSuccess( oldSession, project, mojoExecution, mojo );
>              }
>              catch ( ClassCastException e )
>              {
> @@ -128,10 +143,14 @@ public class DefaultBuildPluginManager
>          }
>          catch ( PluginContainerException e )
>          {
> +            mojoExecutionListener.afterExecutionFailure( oldSession, project, mojoExecution, mojo, e );
> +
>              throw new PluginExecutionException( mojoExecution, project, e );
>          }
>          catch ( NoClassDefFoundError e )
>          {
> +            mojoExecutionListener.afterExecutionFailure( oldSession, project, mojoExecution, mojo, e );
> +
>              ByteArrayOutputStream os = new ByteArrayOutputStream( 1024 );
>              PrintStream ps = new PrintStream( os );
>              ps.println( "A required class was missing while executing " + mojoDescriptor.getId() + ": "
> @@ -144,6 +163,8 @@ public class DefaultBuildPluginManager
>          }
>          catch ( LinkageError e )
>          {
> +            mojoExecutionListener.afterExecutionFailure( oldSession, project, mojoExecution, mojo, e );
> +
>              ByteArrayOutputStream os = new ByteArrayOutputStream( 1024 );
>              PrintStream ps = new PrintStream( os );
>              ps.println( "An API incompatibility was encountered while executing " + mojoDescriptor.getId() + ": "
> @@ -156,6 +177,8 @@ public class DefaultBuildPluginManager
>          }
>          catch ( ClassCastException e )
>          {
> +            mojoExecutionListener.afterExecutionFailure( oldSession, project, mojoExecution, mojo, e );
> +
>              ByteArrayOutputStream os = new ByteArrayOutputStream( 1024 );
>              PrintStream ps = new PrintStream( os );
>              ps.println( "A type incompatibility occured while executing " + mojoDescriptor.getId() + ": "
> @@ -164,10 +187,14 @@ public class DefaultBuildPluginManager
>
>              throw new PluginExecutionException( mojoExecution, project, os.toString(), e );
>          }
> -        finally
> +        catch ( RuntimeException e )
>          {
> -            scope.afterExecutionAlways();
> +            mojoExecutionListener.afterExecutionFailure( oldSession, project, mojoExecution, mojo, e );
>
> +            throw e;
> +        }
> +        finally
> +        {
>              mavenPluginManager.releaseMojo( mojo, mojoExecution );
>
>              scope.exit();
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingMojoExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingMojoExecutionListener.java b/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingMojoExecutionListener.java
> new file mode 100644
> index 0000000..d58329c
> --- /dev/null
> +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingMojoExecutionListener.java
> @@ -0,0 +1,61 @@
> +package org.apache.maven.lifecycle;
> +
> +import java.util.List;
> +import java.util.concurrent.CopyOnWriteArrayList;
> +
> +import javax.inject.Named;
> +import javax.inject.Singleton;
> +
> +import org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.MojoExecutionListener;
> +import org.apache.maven.plugin.Mojo;
> +import org.apache.maven.plugin.MojoExecution;
> +import org.apache.maven.plugin.MojoExecutionException;
> +import org.apache.maven.project.MavenProject;
> +
> +@Named
> +@Singleton
> +public class DelegatingMojoExecutionListener
> +    implements MojoExecutionListener
> +{
> +    private final List<MojoExecutionListener> listeners = new CopyOnWriteArrayList<MojoExecutionListener>();
> +
> +    public void beforeMojoExecution( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo )
> +        throws MojoExecutionException
> +    {
> +        for ( MojoExecutionListener listener : listeners )
> +        {
> +            listener.beforeMojoExecution( session, project, execution, mojo );
> +        }
> +    }
> +
> +    public void afterMojoExecutionSuccess( MavenSession session, MavenProject project, MojoExecution execution,
> +                                           Mojo mojo )
> +        throws MojoExecutionException
> +    {
> +        for ( MojoExecutionListener listener : listeners )
> +        {
> +            listener.afterMojoExecutionSuccess( session, project, execution, mojo );
> +        }
> +    }
> +
> +    public void afterExecutionFailure( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo,
> +                                       Throwable cause )
> +    {
> +        for ( MojoExecutionListener listener : listeners )
> +        {
> +            listener.afterExecutionFailure( session, project, execution, mojo, cause );
> +        }
> +    }
> +
> +    public void addMojoExecutionListener( MojoExecutionListener listener )
> +    {
> +        this.listeners.add( listener );
> +    }
> +
> +    public void removeMojoExecutionListener( MojoExecutionListener listener )
> +    {
> +        this.listeners.remove( listener );
> +    }
> +
> +}
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingProjectExecutionListener.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingProjectExecutionListener.java b/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingProjectExecutionListener.java
> new file mode 100644
> index 0000000..27b53c6
> --- /dev/null
> +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/DelegatingProjectExecutionListener.java
> @@ -0,0 +1,66 @@
> +package org.apache.maven.lifecycle;
> +
> +import java.util.List;
> +import java.util.concurrent.CopyOnWriteArrayList;
> +
> +import javax.inject.Named;
> +import javax.inject.Singleton;
> +
> +import org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.ProjectExecutionListener;
> +import org.apache.maven.plugin.MojoExecution;
> +import org.apache.maven.project.MavenProject;
> +
> +@Named
> +@Singleton
> +public class DelegatingProjectExecutionListener
> +    implements ProjectExecutionListener
> +{
> +    private final List<ProjectExecutionListener> listeners = new CopyOnWriteArrayList<ProjectExecutionListener>();
> +
> +    public void beforeProjectExecution( MavenSession session, MavenProject project )
> +        throws LifecycleExecutionException
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.beforeProjectExecution( session, project );
> +        }
> +    }
> +
> +    public void beforeProjectLifecycleExecution( MavenSession session, MavenProject project,
> +                                                 List<MojoExecution> executionPlan )
> +        throws LifecycleExecutionException
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.beforeProjectLifecycleExecution( session, project, executionPlan );
> +        }
> +    }
> +
> +    public void afterProjectExecutionSuccess( MavenSession session, MavenProject project )
> +        throws LifecycleExecutionException
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.afterProjectExecutionSuccess( session, project );
> +        }
> +    }
> +
> +    public void afterProjectExecutionFailure( MavenSession session, MavenProject project, Throwable cause )
> +    {
> +        for ( ProjectExecutionListener listener : listeners )
> +        {
> +            listener.afterProjectExecutionFailure( session, project, cause );
> +        }
> +    }
> +
> +    public void addProjectExecutionListener( ProjectExecutionListener listener )
> +    {
> +        this.listeners.add( listener );
> +    }
> +
> +    public void removeProjectExecutionListener( ProjectExecutionListener listener )
> +    {
> +        this.listeners.remove( listener );
> +    }
> +}
>
> http://git-wip-us.apache.org/repos/asf/maven/blob/bc84e6c7/maven-core/src/test/java/org/apache/maven/lifecycle/LifecycleExecutorTest.java
> ----------------------------------------------------------------------
> diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/LifecycleExecutorTest.java b/maven-core/src/test/java/org/apache/maven/lifecycle/LifecycleExecutorTest.java
> index 82e7e93..b983e55 100644
> --- a/maven-core/src/test/java/org/apache/maven/lifecycle/LifecycleExecutorTest.java
> +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/LifecycleExecutorTest.java
> @@ -18,11 +18,15 @@ package org.apache.maven.lifecycle;
>  import java.io.File;
>  import java.util.ArrayList;
>  import java.util.Arrays;
> +import java.util.Collections;
>  import java.util.List;
>
>  import org.apache.maven.AbstractCoreMavenComponentTestCase;
>  import org.apache.maven.exception.ExceptionHandler;
>  import org.apache.maven.execution.MavenSession;
> +import org.apache.maven.execution.MojoExecutionListener;
> +import org.apache.maven.execution.ProjectDependencyGraph;
> +import org.apache.maven.execution.ProjectExecutionListener;
>  import org.apache.maven.lifecycle.internal.DefaultLifecycleTaskSegmentCalculator;
>  import org.apache.maven.lifecycle.internal.ExecutionPlanItem;
>  import org.apache.maven.lifecycle.internal.LifecycleExecutionPlanCalculator;
> @@ -31,9 +35,12 @@ import org.apache.maven.lifecycle.internal.LifecycleTaskSegmentCalculator;
>  import org.apache.maven.lifecycle.internal.MojoDescriptorCreator;
>  import org.apache.maven.lifecycle.internal.TaskSegment;
>  import org.apache.maven.model.Plugin;
> +import org.apache.maven.plugin.Mojo;
>  import org.apache.maven.plugin.MojoExecution;
> +import org.apache.maven.plugin.MojoExecutionException;
>  import org.apache.maven.plugin.MojoNotFoundException;
>  import org.apache.maven.plugin.descriptor.MojoDescriptor;
> +import org.apache.maven.project.MavenProject;
>  import org.codehaus.plexus.component.annotations.Requirement;
>  import org.codehaus.plexus.util.xml.Xpp3Dom;
>
> @@ -380,4 +387,124 @@ public class LifecycleExecutorTest
>          assertEquals("1.0", execution.getConfiguration().getChild( "version" ).getAttribute( "default-value" ));
>      }
>
> +    public void testExecutionListeners()
> +        throws Exception
> +    {
> +        final File pom = getProject( "project-basic" );
> +        final MavenSession session = createMavenSession( pom );
> +        session.setProjectDependencyGraph( new ProjectDependencyGraph()
> +        {
> +            public List<MavenProject> getUpstreamProjects( MavenProject project, boolean transitive )
> +            {
> +                return Collections.emptyList();
> +            }
> +
> +            public List<MavenProject> getSortedProjects()
> +            {
> +                return Collections.singletonList( session.getCurrentProject() );
> +            }
> +
> +            public List<MavenProject> getDownstreamProjects( MavenProject project, boolean transitive )
> +            {
> +                return Collections.emptyList();
> +            }
> +        } );
> +
> +        final List<String> log = new ArrayList<String>();
> +
> +        MojoExecutionListener mojoListener = new MojoExecutionListener()
> +        {
> +            public void beforeMojoExecution( MavenSession session, MavenProject project, MojoExecution execution,
> +                                             Mojo mojo )
> +                throws MojoExecutionException
> +            {
> +                log.add( "beforeMojoExecution " + project.getArtifactId() + ":" + execution.getExecutionId() );
> +            }
> +
> +            public void afterMojoExecutionSuccess( MavenSession session, MavenProject project, MojoExecution execution,
> +                                                   Mojo mojo )
> +                throws MojoExecutionException
> +            {
> +                log.add( "afterMojoExecutionSuccess " + project.getArtifactId() + ":" + execution.getExecutionId() );
> +            }
> +
> +            public void afterExecutionFailure( MavenSession session, MavenProject project, MojoExecution execution,
> +                                               Mojo mojo, Throwable cause )
> +            {
> +                log.add( "afterExecutionFailure " + project.getArtifactId() + ":" + execution.getExecutionId() );
> +            }
> +        };
> +        ProjectExecutionListener projectListener = new ProjectExecutionListener()
> +        {
> +            public void beforeProjectExecution( MavenSession session, MavenProject project )
> +                throws LifecycleExecutionException
> +            {
> +                log.add( "beforeProjectExecution " + project.getArtifactId() );
> +            }
> +
> +            public void beforeProjectLifecycleExecution( MavenSession session, MavenProject project,
> +                                                         List<MojoExecution> executionPlan )
> +                throws LifecycleExecutionException
> +            {
> +                log.add( "beforeProjectLifecycleExecution " + project.getArtifactId() );
> +            }
> +
> +            public void afterProjectExecutionSuccess( MavenSession session, MavenProject project )
> +                throws LifecycleExecutionException
> +            {
> +                log.add( "afterProjectExecutionSuccess " + project.getArtifactId() );
> +            }
> +
> +            public void afterProjectExecutionFailure( MavenSession session, MavenProject project, Throwable cause )
> +            {
> +                log.add( "afterProjectExecutionFailure " + project.getArtifactId() );
> +            }
> +        };
> +        lookup( DelegatingProjectExecutionListener.class ).addProjectExecutionListener( projectListener );
> +        lookup( DelegatingMojoExecutionListener.class ).addMojoExecutionListener( mojoListener );
> +
> +        try
> +        {
> +            lifecycleExecutor.execute( session );
> +        }
> +        finally
> +        {
> +            lookup( DelegatingProjectExecutionListener.class ).removeProjectExecutionListener( projectListener );
> +            lookup( DelegatingMojoExecutionListener.class ).removeMojoExecutionListener( mojoListener );
> +        }
> +
> +        List<String> expectedLog = Arrays.asList( "beforeProjectExecution project-basic", //
> +                                                  "beforeProjectLifecycleExecution project-basic", //
> +                                                  "beforeMojoExecution project-basic:default-resources", //
> +                                                  "afterMojoExecutionSuccess project-basic:default-resources", //
> +                                                  "beforeMojoExecution project-basic:default-compile", //
> +                                                  "afterMojoExecutionSuccess project-basic:default-compile", //
> +                                                  "beforeMojoExecution project-basic:default-testResources", //
> +                                                  "afterMojoExecutionSuccess project-basic:default-testResources", //
> +                                                  "beforeMojoExecution project-basic:default-testCompile", //
> +                                                  "afterMojoExecutionSuccess project-basic:default-testCompile", //
> +                                                  "beforeMojoExecution project-basic:default-test", //
> +                                                  "afterMojoExecutionSuccess project-basic:default-test", //
> +                                                  "beforeMojoExecution project-basic:default-jar", //
> +                                                  "afterMojoExecutionSuccess project-basic:default-jar", //
> +                                                  "afterProjectExecutionSuccess project-basic" //
> +        );
> +
> +        assertEventLog( expectedLog, log );
> +    }
> +
> +    private static void assertEventLog( List<String> expectedList, List<String> actualList )
> +    {
> +        assertEquals( toString( expectedList ), toString( actualList ) );
> +    }
> +
> +    private static String toString( List<String> lines )
> +    {
> +        StringBuilder sb = new StringBuilder();
> +        for ( String line : lines )
> +        {
> +            sb.append( line ).append( '\n' );
> +        }
> +        return sb.toString();
> +    }
>  }
>



-- 
Olivier Lamy
Ecetera: http://ecetera.com.au
http://twitter.com/olamy | http://linkedin.com/in/olamy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.

On 12/18/2013, 16:01, Robert Scholte wrote:
> Just an example:
> http://maven.apache.org/maven-release/maven-release-manager/apidocs/org/apache/maven/shared/release/ReleaseManager.html
>
>
> I'm sure nobody suggested to change the method signatures in the
> beginning, but at some moment one had to.
> The result right now is a huge number of overloaded methods.
> Using the Event object is just a way to ensure you won't end up with
> something like this: an ultimate ugly API.


This is a different use case. Clients are not supposed to implement
ReleaseManager, so adding new methods to that interface is backwards
compatible change and is a reasonable way to evolve that API imho.
And, personally, I don't see anything "ugly" about deprecated overloaded
methods meant to provide backwards compatibility with older clients.

--
Regards,
Igor


>
> Robert
>
> Op Wed, 18 Dec 2013 21:32:11 +0100 schreef Igor Fedorenko
> <ig...@ifedorenko.com>:
>
>> Olivier, Robert,
>>
>> Do you insist on event objects or can live with my initial
>> implementation?
>>
>> I am not sure if I explained this clearly enough, but I never suggested
>> changing existing method signatures in the future. If we need to add
>> new parameters or new callback methods in the future, we can introduce
>> new listener interface
>>
>>     MojoExecutionListener2 extends MojoExecutionListener
>>
>> and add new method to the new interface. Clients that implement
>> original MojoExecutionListener will continue to work as is.
>>
>> --
>> Regards,
>> Igor
>>
>> On 12/16/2013, 17:04, Robert Scholte wrote:
>>> There are 2 issues here:
>>> - What if one of the current methods require an extra Object/argument?
>>> - What if we need an extra method.
>>>
>>> The first one can be solved with the suggestion of Olivier. The method
>>> signature will stay we same, we just extend the Event object passed.
>>> For the latter you need to introduce a new interface of require Java8,
>>> which will support default interface implementations.
>>>
>>> Keeping a stable method signature if much more worth to me then the not
>>> directly visible parameters of the event object.
>>> If the current methods require different arguments, I would consider
>>> different Events.
>>>
>>> Robert
>>>
>>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>>> <ig...@ifedorenko.com>:
>>>
>>>>
>>>>
>>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>>
>>>>>>>> +/**
>>>>>>>>> + * Extension point that allows build extensions observe and
>>>>>>>>> possibly
>>>>> veto mojo executions.
>>>>>>>>> + *
>>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>>> + * @since 3.1.2
>>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>>> can be
>>>>> changed or removed without notice.
>>>>>>>>> + */
>>>>>>>>> +public interface MojoExecutionListener
>>>>>>>>> +{
>>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>> +
>>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>>>>> MavenProject project, MojoExecution execution,
>>>>>>>>> +                                           Mojo mojo )
>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>> +
>>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>>> +                                       Throwable cause );
>>>>>>>>> +}
>>>>>>>
>>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>>> with fields for those objects.
>>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>>
>>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>>> signature of the interface.
>>>>>>> ?
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>>> ProjectExecutionEvent names?
>>>>>
>>>>> Sounds good
>>>>>
>>>>
>>>> So I tried this and I am not sure I like the result.
>>>>
>>>> Event objects make it harder to see (at a glance) what parameters are
>>>> provided to the callbacks, especially because not all callbacks have
>>>> the
>>>> same set of parameters. This muddies the API, I think.
>>>>
>>>> Event objects make it possible to add new callback parameters but won't
>>>> help if we need to add new callbacks.
>>>>
>>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>>> evolution path for this API.
>>>>
>>>> What do you think?
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Robert Scholte <rf...@apache.org>.
Just an example:
http://maven.apache.org/maven-release/maven-release-manager/apidocs/org/apache/maven/shared/release/ReleaseManager.html

I'm sure nobody suggested to change the method signatures in the  
beginning, but at some moment one had to.
The result right now is a huge number of overloaded methods.
Using the Event object is just a way to ensure you won't end up with  
something like this: an ultimate ugly API.

Robert

Op Wed, 18 Dec 2013 21:32:11 +0100 schreef Igor Fedorenko  
<ig...@ifedorenko.com>:

> Olivier, Robert,
>
> Do you insist on event objects or can live with my initial  
> implementation?
>
> I am not sure if I explained this clearly enough, but I never suggested
> changing existing method signatures in the future. If we need to add
> new parameters or new callback methods in the future, we can introduce
> new listener interface
>
>     MojoExecutionListener2 extends MojoExecutionListener
>
> and add new method to the new interface. Clients that implement
> original MojoExecutionListener will continue to work as is.
>
> --
> Regards,
> Igor
>
> On 12/16/2013, 17:04, Robert Scholte wrote:
>> There are 2 issues here:
>> - What if one of the current methods require an extra Object/argument?
>> - What if we need an extra method.
>>
>> The first one can be solved with the suggestion of Olivier. The method
>> signature will stay we same, we just extend the Event object passed.
>> For the latter you need to introduce a new interface of require Java8,
>> which will support default interface implementations.
>>
>> Keeping a stable method signature if much more worth to me then the not
>> directly visible parameters of the event object.
>> If the current methods require different arguments, I would consider
>> different Events.
>>
>> Robert
>>
>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>> <ig...@ifedorenko.com>:
>>
>>>
>>>
>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com>  
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>
>>>>>>> +/**
>>>>>>>> + * Extension point that allows build extensions observe and
>>>>>>>> possibly
>>>> veto mojo executions.
>>>>>>>> + *
>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>> + * @since 3.1.2
>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>> can be
>>>> changed or removed without notice.
>>>>>>>> + */
>>>>>>>> +public interface MojoExecutionListener
>>>>>>>> +{
>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>> +        throws MojoExecutionException;
>>>>>>>> +
>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>>>> MavenProject project, MojoExecution execution,
>>>>>>>> +                                           Mojo mojo )
>>>>>>>> +        throws MojoExecutionException;
>>>>>>>> +
>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>> +                                       Throwable cause );
>>>>>>>> +}
>>>>>>
>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>> with fields for those objects.
>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>
>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>> signature of the interface.
>>>>>> ?
>>>>>>
>>>>>>
>>>>>
>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>> ProjectExecutionEvent names?
>>>>
>>>> Sounds good
>>>>
>>>
>>> So I tried this and I am not sure I like the result.
>>>
>>> Event objects make it harder to see (at a glance) what parameters are
>>> provided to the callbacks, especially because not all callbacks have  
>>> the
>>> same set of parameters. This muddies the API, I think.
>>>
>>> Event objects make it possible to add new callback parameters but won't
>>> help if we need to add new callbacks.
>>>
>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>> evolution path for this API.
>>>
>>> What do you think?
>>>
>>> --
>>> Regards,
>>> Igor
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Baptiste Mathus <bm...@batmat.net>.
Well, Maven is already voted to go onto Java6 from now on ;-).

Anyway, as I say, I know this isn't for tomorrow. But there's another
thing, there was not a lot of exciting features attracting developers
between Java 5 and 7 (Coin was too small to really be a major reason for a
mainstream migration).

But the traction from lambda, default methods, better type inference,
date/time api and so on should help migrate a bit quicker.
Let's hope this will happen for some maven components in the next 3 or 4
(and not 10) years if we are optimists.


2013/12/20 Igor Fedorenko <ig...@ifedorenko.com>

> Java 8 isn't out yet... Maven is still limited java 5... so yeah, we'll
> get there... eventually... around 2023 or so.
>
> Please excuse my sarcasm, I just couldn't resist :-)
>
> --
> Regards,
> Igor
>
>
> On 12/20/2013, 9:40, Baptiste Mathus wrote:
>
>> Sure, it'll take time, but the good news is Java8's default methods are
>> certainly going to progressively cause deprecation of that
>> convention/naming convention.
>>
>>
>> 2013/12/20 Igor Fedorenko <ig...@ifedorenko.com>
>>
>>  I don't think this is eclipse-specific, but IBM did analysis of
>>> java-based API evolution and documented the results at eclipse [1].
>>>
>>> The document is quite thorough and I usually use it as a reference when
>>> I need to develop API for long-term maintenance.
>>>
>>> The "2 Convention" is explained in the part 3.
>>>
>>> [1] http://wiki.eclipse.org/Evolving_Java-based_APIs
>>>
>>> --
>>> Regards,
>>> Igor
>>>
>>>
>>> On 12/19/2013, 23:21, Hervé BOUTEMY wrote:
>>>
>>>  IIUC, numbered interfaces + abstract implementations are roughly Eclipse
>>>> way
>>>> of solving this requirement about API evolution, isn't it?
>>>>
>>>> Regards,
>>>>
>>>> Hervé
>>>>
>>>> Le jeudi 19 décembre 2013 07:43:03 Igor Fedorenko a écrit :
>>>>
>>>>  On 12/18/2013, 16:42, Olivier Lamy wrote:
>>>>>
>>>>>  if you need to add new parameters in the future, you won't have to add
>>>>>> this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
>>>>>> it's just ugly but that's my POV) just add those new fields in
>>>>>> MojoExecutionEvent etc...
>>>>>>
>>>>>> Anyway I only talk by experience and you are the guy who write the
>>>>>> code so do as you want.
>>>>>> This API can be here for  a while so if it's easy to enhance it it's
>>>>>> better.
>>>>>>
>>>>>> BTW I just wonder why you don't want to use this pattern with a bean
>>>>>> rather than a method with a lot of parameters.
>>>>>>
>>>>>>
>>>>> API clarity and consistency are the reasons I have slight preference to
>>>>> explicitly pass parameters to callback methods.
>>>>>
>>>>> MojoExecutionListener2 will still be required to introduce new callback
>>>>> methods, and I like having single/consistent API evolution approach
>>>>> that
>>>>> covers all scenarios.
>>>>>
>>>>> When parameters are passed in explicitly, clients do not have to guess
>>>>> what is available to them. It also makes it little easier to reason
>>>>> what
>>>>> API version a client expects just by looking at implements
>>>>> MojoExecutionListener vs MojoExecutionListener2.
>>>>>
>>>>> At any rate, I don't think this matter much in this particular case, so
>>>>> I'll introduce event objects in next couple of days.
>>>>>
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Igor
>>>>>
>>>>>   On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>  Olivier, Robert,
>>>>>>>
>>>>>>> Do you insist on event objects or can live with my initial
>>>>>>> implementation?
>>>>>>>
>>>>>>> I am not sure if I explained this clearly enough, but I never
>>>>>>> suggested
>>>>>>> changing existing method signatures in the future. If we need to add
>>>>>>> new parameters or new callback methods in the future, we can
>>>>>>> introduce
>>>>>>> new listener interface
>>>>>>>
>>>>>>>       MojoExecutionListener2 extends MojoExecutionListener
>>>>>>>
>>>>>>> and add new method to the new interface. Clients that implement
>>>>>>> original MojoExecutionListener will continue to work as is.
>>>>>>>
>>>>>>> --
>>>>>>> Regards,
>>>>>>> Igor
>>>>>>>
>>>>>>> On 12/16/2013, 17:04, Robert Scholte wrote:
>>>>>>>
>>>>>>>  There are 2 issues here:
>>>>>>>> - What if one of the current methods require an extra
>>>>>>>> Object/argument?
>>>>>>>> - What if we need an extra method.
>>>>>>>>
>>>>>>>> The first one can be solved with the suggestion of Olivier. The
>>>>>>>> method
>>>>>>>> signature will stay we same, we just extend the Event object passed.
>>>>>>>> For the latter you need to introduce a new interface of require
>>>>>>>> Java8,
>>>>>>>> which will support default interface implementations.
>>>>>>>>
>>>>>>>> Keeping a stable method signature if much more worth to me then the
>>>>>>>> not
>>>>>>>> directly visible parameters of the event object.
>>>>>>>> If the current methods require different arguments, I would consider
>>>>>>>> different Events.
>>>>>>>>
>>>>>>>> Robert
>>>>>>>>
>>>>>>>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>>>>>>>>
>>>>>>>> <ig...@ifedorenko.com>:
>>>>>>>>
>>>>>>>>  On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>>>>>>
>>>>>>>>>  On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com>
>>>>>>>>>>
>>>>>>>>>>  wrote:
>>>>>>>>>
>>>>>>>>
>>>>    On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>
>>>>>>
>>>>>>>>>>>  +/**
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>   + * Extension point that allows build extensions observe and
>>>>>>>>>>>>>
>>>>>>>>>>>>>> possibly
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>  veto mojo executions.
>>>>>>>>>>
>>>>>>>>>>   + *
>>>>>>>>>>
>>>>>>>>>>>  + * @see WeakMojoExecutionListener
>>>>>>>>>>>>>> + * @since 3.1.2
>>>>>>>>>>>>>> + * @provisional This interface is part of work in progress
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> can be
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>  changed or removed without notice.
>>>>>>>>>>
>>>>>>>>>>   + */
>>>>>>>>>>
>>>>>>>>>>>  +public interface MojoExecutionListener
>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>  MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>>>>
>>>>>>>>>>   +        throws MojoExecutionException;
>>>>>>>>>>
>>>>>>>>>>>  +
>>>>>>>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession
>>>>>>>>>>>>>> session,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>  MavenProject project, MojoExecution execution,
>>>>>>>>>>
>>>>>>>>>>   +                                           Mojo mojo )
>>>>>>>>>>
>>>>>>>>>>>  +        throws MojoExecutionException;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>  MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>>>>
>>>>>>>>>>   +                                       Throwable cause );
>>>>>>>>>>
>>>>>>>>>>>  +}
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>  I wonder if it will be easier for future enhancement to use a
>>>>>>>>>>>> bean
>>>>>>>>>>>> with fields for those objects.
>>>>>>>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>>>>>>>
>>>>>>>>>>>> Maybe will be simpler to add getter to this bean than changing
>>>>>>>>>>>> the
>>>>>>>>>>>> signature of the interface.
>>>>>>>>>>>> ?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>>>>>>>> ProjectExecutionEvent names?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> Sounds good
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> So I tried this and I am not sure I like the result.
>>>>>>>>>
>>>>>>>>> Event objects make it harder to see (at a glance) what parameters
>>>>>>>>> are
>>>>>>>>> provided to the callbacks, especially because not all callbacks
>>>>>>>>> have
>>>>>>>>> the
>>>>>>>>> same set of parameters. This muddies the API, I think.
>>>>>>>>>
>>>>>>>>> Event objects make it possible to add new callback parameters but
>>>>>>>>> won't
>>>>>>>>> help if we need to add new callbacks.
>>>>>>>>>
>>>>>>>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>>>>>>>> evolution path for this API.
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Regards,
>>>>>>>>> Igor
>>>>>>>>>
>>>>>>>>> ------------------------------------------------------------
>>>>>>>>> ---------
>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>>>>
>>>>>>>>>
>>>>>>>> ------------------------------------------------------------
>>>>>>>> ---------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>>>
>>>>>>>>
>>>>>>> ------------------------------------------------------------
>>>>>>> ---------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>>
>>>>>>>
>>>>>>  ------------------------------------------------------------
>>>>> ---------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>>
>>>>  ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>>
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>


-- 
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.
Java 8 isn't out yet... Maven is still limited java 5... so yeah, we'll
get there... eventually... around 2023 or so.

Please excuse my sarcasm, I just couldn't resist :-)

--
Regards,
Igor

On 12/20/2013, 9:40, Baptiste Mathus wrote:
> Sure, it'll take time, but the good news is Java8's default methods are
> certainly going to progressively cause deprecation of that
> convention/naming convention.
>
>
> 2013/12/20 Igor Fedorenko <ig...@ifedorenko.com>
>
>> I don't think this is eclipse-specific, but IBM did analysis of
>> java-based API evolution and documented the results at eclipse [1].
>>
>> The document is quite thorough and I usually use it as a reference when
>> I need to develop API for long-term maintenance.
>>
>> The "2 Convention" is explained in the part 3.
>>
>> [1] http://wiki.eclipse.org/Evolving_Java-based_APIs
>>
>> --
>> Regards,
>> Igor
>>
>>
>> On 12/19/2013, 23:21, Hervé BOUTEMY wrote:
>>
>>> IIUC, numbered interfaces + abstract implementations are roughly Eclipse
>>> way
>>> of solving this requirement about API evolution, isn't it?
>>>
>>> Regards,
>>>
>>> Hervé
>>>
>>> Le jeudi 19 décembre 2013 07:43:03 Igor Fedorenko a écrit :
>>>
>>>> On 12/18/2013, 16:42, Olivier Lamy wrote:
>>>>
>>>>> if you need to add new parameters in the future, you won't have to add
>>>>> this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
>>>>> it's just ugly but that's my POV) just add those new fields in
>>>>> MojoExecutionEvent etc...
>>>>>
>>>>> Anyway I only talk by experience and you are the guy who write the
>>>>> code so do as you want.
>>>>> This API can be here for  a while so if it's easy to enhance it it's
>>>>> better.
>>>>>
>>>>> BTW I just wonder why you don't want to use this pattern with a bean
>>>>> rather than a method with a lot of parameters.
>>>>>
>>>>
>>>> API clarity and consistency are the reasons I have slight preference to
>>>> explicitly pass parameters to callback methods.
>>>>
>>>> MojoExecutionListener2 will still be required to introduce new callback
>>>> methods, and I like having single/consistent API evolution approach that
>>>> covers all scenarios.
>>>>
>>>> When parameters are passed in explicitly, clients do not have to guess
>>>> what is available to them. It also makes it little easier to reason what
>>>> API version a client expects just by looking at implements
>>>> MojoExecutionListener vs MojoExecutionListener2.
>>>>
>>>> At any rate, I don't think this matter much in this particular case, so
>>>> I'll introduce event objects in next couple of days.
>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>>   On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com> wrote:
>>>>>
>>>>>> Olivier, Robert,
>>>>>>
>>>>>> Do you insist on event objects or can live with my initial
>>>>>> implementation?
>>>>>>
>>>>>> I am not sure if I explained this clearly enough, but I never suggested
>>>>>> changing existing method signatures in the future. If we need to add
>>>>>> new parameters or new callback methods in the future, we can introduce
>>>>>> new listener interface
>>>>>>
>>>>>>       MojoExecutionListener2 extends MojoExecutionListener
>>>>>>
>>>>>> and add new method to the new interface. Clients that implement
>>>>>> original MojoExecutionListener will continue to work as is.
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Igor
>>>>>>
>>>>>> On 12/16/2013, 17:04, Robert Scholte wrote:
>>>>>>
>>>>>>> There are 2 issues here:
>>>>>>> - What if one of the current methods require an extra Object/argument?
>>>>>>> - What if we need an extra method.
>>>>>>>
>>>>>>> The first one can be solved with the suggestion of Olivier. The method
>>>>>>> signature will stay we same, we just extend the Event object passed.
>>>>>>> For the latter you need to introduce a new interface of require Java8,
>>>>>>> which will support default interface implementations.
>>>>>>>
>>>>>>> Keeping a stable method signature if much more worth to me then the
>>>>>>> not
>>>>>>> directly visible parameters of the event object.
>>>>>>> If the current methods require different arguments, I would consider
>>>>>>> different Events.
>>>>>>>
>>>>>>> Robert
>>>>>>>
>>>>>>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>>>>>>>
>>>>>>> <ig...@ifedorenko.com>:
>>>>>>>
>>>>>>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>>>>>
>>>>>>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com>
>>>>>>>>>
>>>>>>>> wrote:
>>>
>>>>   On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>>>>
>>>>>>>>>>> +/**
>>>>>>>>>>>>
>>>>>>>>>>>>   + * Extension point that allows build extensions observe and
>>>>>>>>>>>>> possibly
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> veto mojo executions.
>>>>>>>>>
>>>>>>>>>   + *
>>>>>>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>>>>>>> + * @since 3.1.2
>>>>>>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>>>>>>> can be
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> changed or removed without notice.
>>>>>>>>>
>>>>>>>>>   + */
>>>>>>>>>>>>> +public interface MojoExecutionListener
>>>>>>>>>>>>> +{
>>>>>>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>>>
>>>>>>>>>   +        throws MojoExecutionException;
>>>>>>>>>>>>> +
>>>>>>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession
>>>>>>>>>>>>> session,
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> MavenProject project, MojoExecution execution,
>>>>>>>>>
>>>>>>>>>   +                                           Mojo mojo )
>>>>>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>>>>>> +
>>>>>>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>>>
>>>>>>>>>   +                                       Throwable cause );
>>>>>>>>>>>>> +}
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>>>>>>> with fields for those objects.
>>>>>>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>>>>>>
>>>>>>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>>>>>>> signature of the interface.
>>>>>>>>>>> ?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>>>>>>> ProjectExecutionEvent names?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sounds good
>>>>>>>>>
>>>>>>>>
>>>>>>>> So I tried this and I am not sure I like the result.
>>>>>>>>
>>>>>>>> Event objects make it harder to see (at a glance) what parameters are
>>>>>>>> provided to the callbacks, especially because not all callbacks have
>>>>>>>> the
>>>>>>>> same set of parameters. This muddies the API, I think.
>>>>>>>>
>>>>>>>> Event objects make it possible to add new callback parameters but
>>>>>>>> won't
>>>>>>>> help if we need to add new callbacks.
>>>>>>>>
>>>>>>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>>>>>>> evolution path for this API.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> --
>>>>>>>> Regards,
>>>>>>>> Igor
>>>>>>>>
>>>>>>>> ------------------------------------------------------------
>>>>>>>> ---------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>
>>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Baptiste Mathus <bm...@batmat.net>.
Sure, it'll take time, but the good news is Java8's default methods are
certainly going to progressively cause deprecation of that
convention/naming convention.


2013/12/20 Igor Fedorenko <ig...@ifedorenko.com>

> I don't think this is eclipse-specific, but IBM did analysis of
> java-based API evolution and documented the results at eclipse [1].
>
> The document is quite thorough and I usually use it as a reference when
> I need to develop API for long-term maintenance.
>
> The "2 Convention" is explained in the part 3.
>
> [1] http://wiki.eclipse.org/Evolving_Java-based_APIs
>
> --
> Regards,
> Igor
>
>
> On 12/19/2013, 23:21, Hervé BOUTEMY wrote:
>
>> IIUC, numbered interfaces + abstract implementations are roughly Eclipse
>> way
>> of solving this requirement about API evolution, isn't it?
>>
>> Regards,
>>
>> Hervé
>>
>> Le jeudi 19 décembre 2013 07:43:03 Igor Fedorenko a écrit :
>>
>>> On 12/18/2013, 16:42, Olivier Lamy wrote:
>>>
>>>> if you need to add new parameters in the future, you won't have to add
>>>> this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
>>>> it's just ugly but that's my POV) just add those new fields in
>>>> MojoExecutionEvent etc...
>>>>
>>>> Anyway I only talk by experience and you are the guy who write the
>>>> code so do as you want.
>>>> This API can be here for  a while so if it's easy to enhance it it's
>>>> better.
>>>>
>>>> BTW I just wonder why you don't want to use this pattern with a bean
>>>> rather than a method with a lot of parameters.
>>>>
>>>
>>> API clarity and consistency are the reasons I have slight preference to
>>> explicitly pass parameters to callback methods.
>>>
>>> MojoExecutionListener2 will still be required to introduce new callback
>>> methods, and I like having single/consistent API evolution approach that
>>> covers all scenarios.
>>>
>>> When parameters are passed in explicitly, clients do not have to guess
>>> what is available to them. It also makes it little easier to reason what
>>> API version a client expects just by looking at implements
>>> MojoExecutionListener vs MojoExecutionListener2.
>>>
>>> At any rate, I don't think this matter much in this particular case, so
>>> I'll introduce event objects in next couple of days.
>>>
>>>
>>> --
>>> Regards,
>>> Igor
>>>
>>>  On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com> wrote:
>>>>
>>>>> Olivier, Robert,
>>>>>
>>>>> Do you insist on event objects or can live with my initial
>>>>> implementation?
>>>>>
>>>>> I am not sure if I explained this clearly enough, but I never suggested
>>>>> changing existing method signatures in the future. If we need to add
>>>>> new parameters or new callback methods in the future, we can introduce
>>>>> new listener interface
>>>>>
>>>>>      MojoExecutionListener2 extends MojoExecutionListener
>>>>>
>>>>> and add new method to the new interface. Clients that implement
>>>>> original MojoExecutionListener will continue to work as is.
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Igor
>>>>>
>>>>> On 12/16/2013, 17:04, Robert Scholte wrote:
>>>>>
>>>>>> There are 2 issues here:
>>>>>> - What if one of the current methods require an extra Object/argument?
>>>>>> - What if we need an extra method.
>>>>>>
>>>>>> The first one can be solved with the suggestion of Olivier. The method
>>>>>> signature will stay we same, we just extend the Event object passed.
>>>>>> For the latter you need to introduce a new interface of require Java8,
>>>>>> which will support default interface implementations.
>>>>>>
>>>>>> Keeping a stable method signature if much more worth to me then the
>>>>>> not
>>>>>> directly visible parameters of the event object.
>>>>>> If the current methods require different arguments, I would consider
>>>>>> different Events.
>>>>>>
>>>>>> Robert
>>>>>>
>>>>>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>>>>>>
>>>>>> <ig...@ifedorenko.com>:
>>>>>>
>>>>>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>>>>
>>>>>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com>
>>>>>>>>
>>>>>>> wrote:
>>
>>>  On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>>>
>>>>>>>>>> +/**
>>>>>>>>>>>
>>>>>>>>>>>  + * Extension point that allows build extensions observe and
>>>>>>>>>>>> possibly
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>> veto mojo executions.
>>>>>>>>
>>>>>>>>  + *
>>>>>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>>>>>> + * @since 3.1.2
>>>>>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>>>>>> can be
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>> changed or removed without notice.
>>>>>>>>
>>>>>>>>  + */
>>>>>>>>>>>> +public interface MojoExecutionListener
>>>>>>>>>>>> +{
>>>>>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>>
>>>>>>>>  +        throws MojoExecutionException;
>>>>>>>>>>>> +
>>>>>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession
>>>>>>>>>>>> session,
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>> MavenProject project, MojoExecution execution,
>>>>>>>>
>>>>>>>>  +                                           Mojo mojo )
>>>>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>>>>> +
>>>>>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>>
>>>>>>>>  +                                       Throwable cause );
>>>>>>>>>>>> +}
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>>>>>> with fields for those objects.
>>>>>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>>>>>
>>>>>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>>>>>> signature of the interface.
>>>>>>>>>> ?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>>>>>> ProjectExecutionEvent names?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Sounds good
>>>>>>>>
>>>>>>>
>>>>>>> So I tried this and I am not sure I like the result.
>>>>>>>
>>>>>>> Event objects make it harder to see (at a glance) what parameters are
>>>>>>> provided to the callbacks, especially because not all callbacks have
>>>>>>> the
>>>>>>> same set of parameters. This muddies the API, I think.
>>>>>>>
>>>>>>> Event objects make it possible to add new callback parameters but
>>>>>>> won't
>>>>>>> help if we need to add new callbacks.
>>>>>>>
>>>>>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>>>>>> evolution path for this API.
>>>>>>>
>>>>>>> What do you think?
>>>>>>>
>>>>>>> --
>>>>>>> Regards,
>>>>>>> Igor
>>>>>>>
>>>>>>> ------------------------------------------------------------
>>>>>>> ---------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>
>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>


-- 
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.
I don't think this is eclipse-specific, but IBM did analysis of
java-based API evolution and documented the results at eclipse [1].

The document is quite thorough and I usually use it as a reference when
I need to develop API for long-term maintenance.

The "2 Convention" is explained in the part 3.

[1] http://wiki.eclipse.org/Evolving_Java-based_APIs

--
Regards,
Igor

On 12/19/2013, 23:21, Hervé BOUTEMY wrote:
> IIUC, numbered interfaces + abstract implementations are roughly Eclipse way
> of solving this requirement about API evolution, isn't it?
>
> Regards,
>
> Hervé
>
> Le jeudi 19 décembre 2013 07:43:03 Igor Fedorenko a écrit :
>> On 12/18/2013, 16:42, Olivier Lamy wrote:
>>> if you need to add new parameters in the future, you won't have to add
>>> this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
>>> it's just ugly but that's my POV) just add those new fields in
>>> MojoExecutionEvent etc...
>>>
>>> Anyway I only talk by experience and you are the guy who write the
>>> code so do as you want.
>>> This API can be here for  a while so if it's easy to enhance it it's
>>> better.
>>>
>>> BTW I just wonder why you don't want to use this pattern with a bean
>>> rather than a method with a lot of parameters.
>>
>> API clarity and consistency are the reasons I have slight preference to
>> explicitly pass parameters to callback methods.
>>
>> MojoExecutionListener2 will still be required to introduce new callback
>> methods, and I like having single/consistent API evolution approach that
>> covers all scenarios.
>>
>> When parameters are passed in explicitly, clients do not have to guess
>> what is available to them. It also makes it little easier to reason what
>> API version a client expects just by looking at implements
>> MojoExecutionListener vs MojoExecutionListener2.
>>
>> At any rate, I don't think this matter much in this particular case, so
>> I'll introduce event objects in next couple of days.
>>
>>
>> --
>> Regards,
>> Igor
>>
>>> On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com> wrote:
>>>> Olivier, Robert,
>>>>
>>>> Do you insist on event objects or can live with my initial
>>>> implementation?
>>>>
>>>> I am not sure if I explained this clearly enough, but I never suggested
>>>> changing existing method signatures in the future. If we need to add
>>>> new parameters or new callback methods in the future, we can introduce
>>>> new listener interface
>>>>
>>>>      MojoExecutionListener2 extends MojoExecutionListener
>>>>
>>>> and add new method to the new interface. Clients that implement
>>>> original MojoExecutionListener will continue to work as is.
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>> On 12/16/2013, 17:04, Robert Scholte wrote:
>>>>> There are 2 issues here:
>>>>> - What if one of the current methods require an extra Object/argument?
>>>>> - What if we need an extra method.
>>>>>
>>>>> The first one can be solved with the suggestion of Olivier. The method
>>>>> signature will stay we same, we just extend the Event object passed.
>>>>> For the latter you need to introduce a new interface of require Java8,
>>>>> which will support default interface implementations.
>>>>>
>>>>> Keeping a stable method signature if much more worth to me then the not
>>>>> directly visible parameters of the event object.
>>>>> If the current methods require different arguments, I would consider
>>>>> different Events.
>>>>>
>>>>> Robert
>>>>>
>>>>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>>>>>
>>>>> <ig...@ifedorenko.com>:
>>>>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com>
> wrote:
>>>>>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>>>> +/**
>>>>>>>>>>
>>>>>>>>>>> + * Extension point that allows build extensions observe and
>>>>>>>>>>> possibly
>>>>>>>
>>>>>>> veto mojo executions.
>>>>>>>
>>>>>>>>>>> + *
>>>>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>>>>> + * @since 3.1.2
>>>>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>>>>> can be
>>>>>>>
>>>>>>> changed or removed without notice.
>>>>>>>
>>>>>>>>>>> + */
>>>>>>>>>>> +public interface MojoExecutionListener
>>>>>>>>>>> +{
>>>>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>>>>
>>>>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>
>>>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>>>> +
>>>>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>>>>>>>
>>>>>>> MavenProject project, MojoExecution execution,
>>>>>>>
>>>>>>>>>>> +                                           Mojo mojo )
>>>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>>>> +
>>>>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>>>>
>>>>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>
>>>>>>>>>>> +                                       Throwable cause );
>>>>>>>>>>> +}
>>>>>>>>>
>>>>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>>>>> with fields for those objects.
>>>>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>>>>
>>>>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>>>>> signature of the interface.
>>>>>>>>> ?
>>>>>>>>
>>>>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>>>>> ProjectExecutionEvent names?
>>>>>>>
>>>>>>> Sounds good
>>>>>>
>>>>>> So I tried this and I am not sure I like the result.
>>>>>>
>>>>>> Event objects make it harder to see (at a glance) what parameters are
>>>>>> provided to the callbacks, especially because not all callbacks have
>>>>>> the
>>>>>> same set of parameters. This muddies the API, I think.
>>>>>>
>>>>>> Event objects make it possible to add new callback parameters but won't
>>>>>> help if we need to add new callbacks.
>>>>>>
>>>>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>>>>> evolution path for this API.
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Igor
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Hervé BOUTEMY <he...@free.fr>.
IIUC, numbered interfaces + abstract implementations are roughly Eclipse way 
of solving this requirement about API evolution, isn't it?

Regards,

Hervé

Le jeudi 19 décembre 2013 07:43:03 Igor Fedorenko a écrit :
> On 12/18/2013, 16:42, Olivier Lamy wrote:
> > if you need to add new parameters in the future, you won't have to add
> > this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
> > it's just ugly but that's my POV) just add those new fields in
> > MojoExecutionEvent etc...
> > 
> > Anyway I only talk by experience and you are the guy who write the
> > code so do as you want.
> > This API can be here for  a while so if it's easy to enhance it it's
> > better.
> > 
> > BTW I just wonder why you don't want to use this pattern with a bean
> > rather than a method with a lot of parameters.
> 
> API clarity and consistency are the reasons I have slight preference to
> explicitly pass parameters to callback methods.
> 
> MojoExecutionListener2 will still be required to introduce new callback
> methods, and I like having single/consistent API evolution approach that
> covers all scenarios.
> 
> When parameters are passed in explicitly, clients do not have to guess
> what is available to them. It also makes it little easier to reason what
> API version a client expects just by looking at implements
> MojoExecutionListener vs MojoExecutionListener2.
> 
> At any rate, I don't think this matter much in this particular case, so
> I'll introduce event objects in next couple of days.
> 
> 
> --
> Regards,
> Igor
> 
> > On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com> wrote:
> >> Olivier, Robert,
> >> 
> >> Do you insist on event objects or can live with my initial
> >> implementation?
> >> 
> >> I am not sure if I explained this clearly enough, but I never suggested
> >> changing existing method signatures in the future. If we need to add
> >> new parameters or new callback methods in the future, we can introduce
> >> new listener interface
> >> 
> >>     MojoExecutionListener2 extends MojoExecutionListener
> >> 
> >> and add new method to the new interface. Clients that implement
> >> original MojoExecutionListener will continue to work as is.
> >> 
> >> --
> >> Regards,
> >> Igor
> >> 
> >> On 12/16/2013, 17:04, Robert Scholte wrote:
> >>> There are 2 issues here:
> >>> - What if one of the current methods require an extra Object/argument?
> >>> - What if we need an extra method.
> >>> 
> >>> The first one can be solved with the suggestion of Olivier. The method
> >>> signature will stay we same, we just extend the Event object passed.
> >>> For the latter you need to introduce a new interface of require Java8,
> >>> which will support default interface implementations.
> >>> 
> >>> Keeping a stable method signature if much more worth to me then the not
> >>> directly visible parameters of the event object.
> >>> If the current methods require different arguments, I would consider
> >>> different Events.
> >>> 
> >>> Robert
> >>> 
> >>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
> >>> 
> >>> <ig...@ifedorenko.com>:
> >>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
> >>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> 
wrote:
> >>>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
> >>>>>>>> +/**
> >>>>>>>> 
> >>>>>>>>> + * Extension point that allows build extensions observe and
> >>>>>>>>> possibly
> >>>>> 
> >>>>> veto mojo executions.
> >>>>> 
> >>>>>>>>> + *
> >>>>>>>>> + * @see WeakMojoExecutionListener
> >>>>>>>>> + * @since 3.1.2
> >>>>>>>>> + * @provisional This interface is part of work in progress and
> >>>>>>>>> can be
> >>>>> 
> >>>>> changed or removed without notice.
> >>>>> 
> >>>>>>>>> + */
> >>>>>>>>> +public interface MojoExecutionListener
> >>>>>>>>> +{
> >>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
> >>>>> 
> >>>>> MavenProject project, MojoExecution execution, Mojo mojo )
> >>>>> 
> >>>>>>>>> +        throws MojoExecutionException;
> >>>>>>>>> +
> >>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
> >>>>> 
> >>>>> MavenProject project, MojoExecution execution,
> >>>>> 
> >>>>>>>>> +                                           Mojo mojo )
> >>>>>>>>> +        throws MojoExecutionException;
> >>>>>>>>> +
> >>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
> >>>>> 
> >>>>> MavenProject project, MojoExecution execution, Mojo mojo,
> >>>>> 
> >>>>>>>>> +                                       Throwable cause );
> >>>>>>>>> +}
> >>>>>>> 
> >>>>>>> I wonder if it will be easier for future enhancement to use a bean
> >>>>>>> with fields for those objects.
> >>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
> >>>>>>> 
> >>>>>>> Maybe will be simpler to add getter to this bean than changing the
> >>>>>>> signature of the interface.
> >>>>>>> ?
> >>>>>> 
> >>>>>> Good idea. Any objections against MojoExecutionEvent and
> >>>>>> ProjectExecutionEvent names?
> >>>>> 
> >>>>> Sounds good
> >>>> 
> >>>> So I tried this and I am not sure I like the result.
> >>>> 
> >>>> Event objects make it harder to see (at a glance) what parameters are
> >>>> provided to the callbacks, especially because not all callbacks have
> >>>> the
> >>>> same set of parameters. This muddies the API, I think.
> >>>> 
> >>>> Event objects make it possible to add new callback parameters but won't
> >>>> help if we need to add new callbacks.
> >>>> 
> >>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
> >>>> evolution path for this API.
> >>>> 
> >>>> What do you think?
> >>>> 
> >>>> --
> >>>> Regards,
> >>>> Igor
> >>>> 
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>>> For additional commands, e-mail: dev-help@maven.apache.org
> >>> 
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>> For additional commands, e-mail: dev-help@maven.apache.org
> >> 
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.
So I introduced MojoExecutionEvent and ProjectExecutionEvent [1].

As I already explained, this makes it somewhat hard(er) to know what
parameters each of the callback methods gets. Anyone who implements
these interfaces will have to look inside maven core to see what gets
passed in.

[1] 
https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=e698ce6e67f10df1d830d22e9b0143c6b5549f79

--
Regards,
Igor

On 12/18/2013, 16:42, Olivier Lamy wrote:
> if you need to add new parameters in the future, you won't have to add
> this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
> it's just ugly but that's my POV) just add those new fields in
> MojoExecutionEvent etc...
>
> Anyway I only talk by experience and you are the guy who write the
> code so do as you want.
> This API can be here for  a while so if it's easy to enhance it it's better.
>
> BTW I just wonder why you don't want to use this pattern with a bean
> rather than a method with a lot of parameters.
>
>
>
> On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com> wrote:
>> Olivier, Robert,
>>
>> Do you insist on event objects or can live with my initial implementation?
>>
>> I am not sure if I explained this clearly enough, but I never suggested
>> changing existing method signatures in the future. If we need to add
>> new parameters or new callback methods in the future, we can introduce
>> new listener interface
>>
>>     MojoExecutionListener2 extends MojoExecutionListener
>>
>> and add new method to the new interface. Clients that implement
>> original MojoExecutionListener will continue to work as is.
>>
>> --
>> Regards,
>> Igor
>>
>>
>> On 12/16/2013, 17:04, Robert Scholte wrote:
>>>
>>> There are 2 issues here:
>>> - What if one of the current methods require an extra Object/argument?
>>> - What if we need an extra method.
>>>
>>> The first one can be solved with the suggestion of Olivier. The method
>>> signature will stay we same, we just extend the Event object passed.
>>> For the latter you need to introduce a new interface of require Java8,
>>> which will support default interface implementations.
>>>
>>> Keeping a stable method signature if much more worth to me then the not
>>> directly visible parameters of the event object.
>>> If the current methods require different arguments, I would consider
>>> different Events.
>>>
>>> Robert
>>>
>>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>>> <ig...@ifedorenko.com>:
>>>
>>>>
>>>>
>>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>>
>>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> +/**
>>>>>>>>>
>>>>>>>>> + * Extension point that allows build extensions observe and
>>>>>>>>> possibly
>>>>>
>>>>> veto mojo executions.
>>>>>>>>>
>>>>>>>>> + *
>>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>>> + * @since 3.1.2
>>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>>> can be
>>>>>
>>>>> changed or removed without notice.
>>>>>>>>>
>>>>>>>>> + */
>>>>>>>>> +public interface MojoExecutionListener
>>>>>>>>> +{
>>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>>
>>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>>>
>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>> +
>>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>>>>>
>>>>> MavenProject project, MojoExecution execution,
>>>>>>>>>
>>>>>>>>> +                                           Mojo mojo )
>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>> +
>>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>>
>>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>>>
>>>>>>>>> +                                       Throwable cause );
>>>>>>>>> +}
>>>>>>>
>>>>>>>
>>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>>> with fields for those objects.
>>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>>
>>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>>> signature of the interface.
>>>>>>> ?
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>>> ProjectExecutionEvent names?
>>>>>
>>>>>
>>>>> Sounds good
>>>>>
>>>>
>>>> So I tried this and I am not sure I like the result.
>>>>
>>>> Event objects make it harder to see (at a glance) what parameters are
>>>> provided to the callbacks, especially because not all callbacks have the
>>>> same set of parameters. This muddies the API, I think.
>>>>
>>>> Event objects make it possible to add new callback parameters but won't
>>>> help if we need to add new callbacks.
>>>>
>>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>>> evolution path for this API.
>>>>
>>>> What do you think?
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.

On 12/18/2013, 16:42, Olivier Lamy wrote:
> if you need to add new parameters in the future, you won't have to add
> this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
> it's just ugly but that's my POV) just add those new fields in
> MojoExecutionEvent etc...
>
> Anyway I only talk by experience and you are the guy who write the
> code so do as you want.
> This API can be here for  a while so if it's easy to enhance it it's better.
>
> BTW I just wonder why you don't want to use this pattern with a bean
> rather than a method with a lot of parameters.
>

API clarity and consistency are the reasons I have slight preference to
explicitly pass parameters to callback methods.

MojoExecutionListener2 will still be required to introduce new callback
methods, and I like having single/consistent API evolution approach that
covers all scenarios.

When parameters are passed in explicitly, clients do not have to guess
what is available to them. It also makes it little easier to reason what
API version a client expects just by looking at implements
MojoExecutionListener vs MojoExecutionListener2.

At any rate, I don't think this matter much in this particular case, so
I'll introduce event objects in next couple of days.


--
Regards,
Igor


>
>
> On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com> wrote:
>> Olivier, Robert,
>>
>> Do you insist on event objects or can live with my initial implementation?
>>
>> I am not sure if I explained this clearly enough, but I never suggested
>> changing existing method signatures in the future. If we need to add
>> new parameters or new callback methods in the future, we can introduce
>> new listener interface
>>
>>     MojoExecutionListener2 extends MojoExecutionListener
>>
>> and add new method to the new interface. Clients that implement
>> original MojoExecutionListener will continue to work as is.
>>
>> --
>> Regards,
>> Igor
>>
>>
>> On 12/16/2013, 17:04, Robert Scholte wrote:
>>>
>>> There are 2 issues here:
>>> - What if one of the current methods require an extra Object/argument?
>>> - What if we need an extra method.
>>>
>>> The first one can be solved with the suggestion of Olivier. The method
>>> signature will stay we same, we just extend the Event object passed.
>>> For the latter you need to introduce a new interface of require Java8,
>>> which will support default interface implementations.
>>>
>>> Keeping a stable method signature if much more worth to me then the not
>>> directly visible parameters of the event object.
>>> If the current methods require different arguments, I would consider
>>> different Events.
>>>
>>> Robert
>>>
>>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>>> <ig...@ifedorenko.com>:
>>>
>>>>
>>>>
>>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>>
>>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> +/**
>>>>>>>>>
>>>>>>>>> + * Extension point that allows build extensions observe and
>>>>>>>>> possibly
>>>>>
>>>>> veto mojo executions.
>>>>>>>>>
>>>>>>>>> + *
>>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>>> + * @since 3.1.2
>>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>>> can be
>>>>>
>>>>> changed or removed without notice.
>>>>>>>>>
>>>>>>>>> + */
>>>>>>>>> +public interface MojoExecutionListener
>>>>>>>>> +{
>>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>>
>>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>>>
>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>> +
>>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>>>>>
>>>>> MavenProject project, MojoExecution execution,
>>>>>>>>>
>>>>>>>>> +                                           Mojo mojo )
>>>>>>>>> +        throws MojoExecutionException;
>>>>>>>>> +
>>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>>
>>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>>>
>>>>>>>>> +                                       Throwable cause );
>>>>>>>>> +}
>>>>>>>
>>>>>>>
>>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>>> with fields for those objects.
>>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>>
>>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>>> signature of the interface.
>>>>>>> ?
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>>> ProjectExecutionEvent names?
>>>>>
>>>>>
>>>>> Sounds good
>>>>>
>>>>
>>>> So I tried this and I am not sure I like the result.
>>>>
>>>> Event objects make it harder to see (at a glance) what parameters are
>>>> provided to the callbacks, especially because not all callbacks have the
>>>> same set of parameters. This muddies the API, I think.
>>>>
>>>> Event objects make it possible to add new callback parameters but won't
>>>> help if we need to add new callbacks.
>>>>
>>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>>> evolution path for this API.
>>>>
>>>> What do you think?
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Olivier Lamy <ol...@apache.org>.
if you need to add new parameters in the future, you won't have to add
this new MojoExecutionListener2 extends MojoExecutionListener (IMHO
it's just ugly but that's my POV) just add those new fields in
MojoExecutionEvent etc...

Anyway I only talk by experience and you are the guy who write the
code so do as you want.
This API can be here for  a while so if it's easy to enhance it it's better.

BTW I just wonder why you don't want to use this pattern with a bean
rather than a method with a lot of parameters.



On 19 December 2013 07:32, Igor Fedorenko <ig...@ifedorenko.com> wrote:
> Olivier, Robert,
>
> Do you insist on event objects or can live with my initial implementation?
>
> I am not sure if I explained this clearly enough, but I never suggested
> changing existing method signatures in the future. If we need to add
> new parameters or new callback methods in the future, we can introduce
> new listener interface
>
>    MojoExecutionListener2 extends MojoExecutionListener
>
> and add new method to the new interface. Clients that implement
> original MojoExecutionListener will continue to work as is.
>
> --
> Regards,
> Igor
>
>
> On 12/16/2013, 17:04, Robert Scholte wrote:
>>
>> There are 2 issues here:
>> - What if one of the current methods require an extra Object/argument?
>> - What if we need an extra method.
>>
>> The first one can be solved with the suggestion of Olivier. The method
>> signature will stay we same, we just extend the Event object passed.
>> For the latter you need to introduce a new interface of require Java8,
>> which will support default interface implementations.
>>
>> Keeping a stable method signature if much more worth to me then the not
>> directly visible parameters of the event object.
>> If the current methods require different arguments, I would consider
>> different Events.
>>
>> Robert
>>
>> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
>> <ig...@ifedorenko.com>:
>>
>>>
>>>
>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>>
>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>>
>>>>>>>
>>>>>>> +/**
>>>>>>>>
>>>>>>>> + * Extension point that allows build extensions observe and
>>>>>>>> possibly
>>>>
>>>> veto mojo executions.
>>>>>>>>
>>>>>>>> + *
>>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>>> + * @since 3.1.2
>>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>>> can be
>>>>
>>>> changed or removed without notice.
>>>>>>>>
>>>>>>>> + */
>>>>>>>> +public interface MojoExecutionListener
>>>>>>>> +{
>>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>>>
>>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>>>
>>>>>>>> +        throws MojoExecutionException;
>>>>>>>> +
>>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>>>>
>>>> MavenProject project, MojoExecution execution,
>>>>>>>>
>>>>>>>> +                                           Mojo mojo )
>>>>>>>> +        throws MojoExecutionException;
>>>>>>>> +
>>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>>>
>>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>>>
>>>>>>>> +                                       Throwable cause );
>>>>>>>> +}
>>>>>>
>>>>>>
>>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>>> with fields for those objects.
>>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>>
>>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>>> signature of the interface.
>>>>>> ?
>>>>>>
>>>>>>
>>>>>
>>>>> Good idea. Any objections against MojoExecutionEvent and
>>>>> ProjectExecutionEvent names?
>>>>
>>>>
>>>> Sounds good
>>>>
>>>
>>> So I tried this and I am not sure I like the result.
>>>
>>> Event objects make it harder to see (at a glance) what parameters are
>>> provided to the callbacks, especially because not all callbacks have the
>>> same set of parameters. This muddies the API, I think.
>>>
>>> Event objects make it possible to add new callback parameters but won't
>>> help if we need to add new callbacks.
>>>
>>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>>> evolution path for this API.
>>>
>>> What do you think?
>>>
>>> --
>>> Regards,
>>> Igor
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>



-- 
Olivier Lamy
Ecetera: http://ecetera.com.au
http://twitter.com/olamy | http://linkedin.com/in/olamy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.
Olivier, Robert,

Do you insist on event objects or can live with my initial implementation?

I am not sure if I explained this clearly enough, but I never suggested
changing existing method signatures in the future. If we need to add
new parameters or new callback methods in the future, we can introduce
new listener interface

    MojoExecutionListener2 extends MojoExecutionListener

and add new method to the new interface. Clients that implement
original MojoExecutionListener will continue to work as is.

--
Regards,
Igor

On 12/16/2013, 17:04, Robert Scholte wrote:
> There are 2 issues here:
> - What if one of the current methods require an extra Object/argument?
> - What if we need an extra method.
>
> The first one can be solved with the suggestion of Olivier. The method
> signature will stay we same, we just extend the Event object passed.
> For the latter you need to introduce a new interface of require Java8,
> which will support default interface implementations.
>
> Keeping a stable method signature if much more worth to me then the not
> directly visible parameters of the event object.
> If the current methods require different arguments, I would consider
> different Events.
>
> Robert
>
> Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko
> <ig...@ifedorenko.com>:
>
>>
>>
>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> wrote:
>>>>
>>>>
>>>>
>>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>>
>>>>>> +/**
>>>>>>> + * Extension point that allows build extensions observe and
>>>>>>> possibly
>>> veto mojo executions.
>>>>>>> + *
>>>>>>> + * @see WeakMojoExecutionListener
>>>>>>> + * @since 3.1.2
>>>>>>> + * @provisional This interface is part of work in progress and
>>>>>>> can be
>>> changed or removed without notice.
>>>>>>> + */
>>>>>>> +public interface MojoExecutionListener
>>>>>>> +{
>>>>>>> +    public void beforeMojoExecution( MavenSession session,
>>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>>> +        throws MojoExecutionException;
>>>>>>> +
>>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>>> MavenProject project, MojoExecution execution,
>>>>>>> +                                           Mojo mojo )
>>>>>>> +        throws MojoExecutionException;
>>>>>>> +
>>>>>>> +    public void afterExecutionFailure( MavenSession session,
>>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>>> +                                       Throwable cause );
>>>>>>> +}
>>>>>
>>>>> I wonder if it will be easier for future enhancement to use a bean
>>>>> with fields for those objects.
>>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>>
>>>>> Maybe will be simpler to add getter to this bean than changing the
>>>>> signature of the interface.
>>>>> ?
>>>>>
>>>>>
>>>>
>>>> Good idea. Any objections against MojoExecutionEvent and
>>>> ProjectExecutionEvent names?
>>>
>>> Sounds good
>>>
>>
>> So I tried this and I am not sure I like the result.
>>
>> Event objects make it harder to see (at a glance) what parameters are
>> provided to the callbacks, especially because not all callbacks have the
>> same set of parameters. This muddies the API, I think.
>>
>> Event objects make it possible to add new callback parameters but won't
>> help if we need to add new callbacks.
>>
>> I think MojoExecutionListener2/3/4/etc provides reasonably good
>> evolution path for this API.
>>
>> What do you think?
>>
>> --
>> Regards,
>> Igor
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Robert Scholte <rf...@apache.org>.
There are 2 issues here:
- What if one of the current methods require an extra Object/argument?
- What if we need an extra method.

The first one can be solved with the suggestion of Olivier. The method  
signature will stay we same, we just extend the Event object passed.
For the latter you need to introduce a new interface of require Java8,  
which will support default interface implementations.

Keeping a stable method signature if much more worth to me then the not  
directly visible parameters of the event object.
If the current methods require different arguments, I would consider  
different Events.

Robert

Op Mon, 16 Dec 2013 16:45:14 +0100 schreef Igor Fedorenko  
<ig...@ifedorenko.com>:

>
>
> On 12/16/2013, 7:39, Olivier Lamy wrote:
>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> wrote:
>>>
>>>
>>>
>>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>>
>>>>> +/**
>>>>>> + * Extension point that allows build extensions observe and  
>>>>>> possibly
>> veto mojo executions.
>>>>>> + *
>>>>>> + * @see WeakMojoExecutionListener
>>>>>> + * @since 3.1.2
>>>>>> + * @provisional This interface is part of work in progress and can  
>>>>>> be
>> changed or removed without notice.
>>>>>> + */
>>>>>> +public interface MojoExecutionListener
>>>>>> +{
>>>>>> +    public void beforeMojoExecution( MavenSession session,
>> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>>> +        throws MojoExecutionException;
>>>>>> +
>>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
>> MavenProject project, MojoExecution execution,
>>>>>> +                                           Mojo mojo )
>>>>>> +        throws MojoExecutionException;
>>>>>> +
>>>>>> +    public void afterExecutionFailure( MavenSession session,
>> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>>> +                                       Throwable cause );
>>>>>> +}
>>>>
>>>> I wonder if it will be easier for future enhancement to use a bean
>>>> with fields for those objects.
>>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>>
>>>> Maybe will be simpler to add getter to this bean than changing the
>>>> signature of the interface.
>>>> ?
>>>>
>>>>
>>>
>>> Good idea. Any objections against MojoExecutionEvent and
>>> ProjectExecutionEvent names?
>>
>> Sounds good
>>
>
> So I tried this and I am not sure I like the result.
>
> Event objects make it harder to see (at a glance) what parameters are
> provided to the callbacks, especially because not all callbacks have the
> same set of parameters. This muddies the API, I think.
>
> Event objects make it possible to add new callback parameters but won't
> help if we need to add new callbacks.
>
> I think MojoExecutionListener2/3/4/etc provides reasonably good
> evolution path for this API.
>
> What do you think?
>
> --
> Regards,
> Igor
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.

On 12/16/2013, 7:39, Olivier Lamy wrote:
> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> wrote:
>>
>>
>>
>> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>>
>>>> +/**
>>>>> + * Extension point that allows build extensions observe and possibly
> veto mojo executions.
>>>>> + *
>>>>> + * @see WeakMojoExecutionListener
>>>>> + * @since 3.1.2
>>>>> + * @provisional This interface is part of work in progress and can be
> changed or removed without notice.
>>>>> + */
>>>>> +public interface MojoExecutionListener
>>>>> +{
>>>>> +    public void beforeMojoExecution( MavenSession session,
> MavenProject project, MojoExecution execution, Mojo mojo )
>>>>> +        throws MojoExecutionException;
>>>>> +
>>>>> +    public void afterMojoExecutionSuccess( MavenSession session,
> MavenProject project, MojoExecution execution,
>>>>> +                                           Mojo mojo )
>>>>> +        throws MojoExecutionException;
>>>>> +
>>>>> +    public void afterExecutionFailure( MavenSession session,
> MavenProject project, MojoExecution execution, Mojo mojo,
>>>>> +                                       Throwable cause );
>>>>> +}
>>>
>>> I wonder if it will be easier for future enhancement to use a bean
>>> with fields for those objects.
>>> MojoExecutionListenerEvent with getMavenSession() etc...
>>>
>>> Maybe will be simpler to add getter to this bean than changing the
>>> signature of the interface.
>>> ?
>>>
>>>
>>
>> Good idea. Any objections against MojoExecutionEvent and
>> ProjectExecutionEvent names?
>
> Sounds good
>

So I tried this and I am not sure I like the result.

Event objects make it harder to see (at a glance) what parameters are
provided to the callbacks, especially because not all callbacks have the
same set of parameters. This muddies the API, I think.

Event objects make it possible to add new callback parameters but won't
help if we need to add new callbacks.

I think MojoExecutionListener2/3/4/etc provides reasonably good
evolution path for this API.

What do you think?

--
Regards,
Igor

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Olivier Lamy <ol...@apache.org>.
On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <ig...@ifedorenko.com> wrote:
>
>
>
> On 12/16/2013, 5:27, Olivier Lamy wrote:
>>>
>>> +/**
>>> >+ * Extension point that allows build extensions observe and possibly
veto mojo executions.
>>> >+ *
>>> >+ * @see WeakMojoExecutionListener
>>> >+ * @since 3.1.2
>>> >+ * @provisional This interface is part of work in progress and can be
changed or removed without notice.
>>> >+ */
>>> >+public interface MojoExecutionListener
>>> >+{
>>> >+    public void beforeMojoExecution( MavenSession session,
MavenProject project, MojoExecution execution, Mojo mojo )
>>> >+        throws MojoExecutionException;
>>> >+
>>> >+    public void afterMojoExecutionSuccess( MavenSession session,
MavenProject project, MojoExecution execution,
>>> >+                                           Mojo mojo )
>>> >+        throws MojoExecutionException;
>>> >+
>>> >+    public void afterExecutionFailure( MavenSession session,
MavenProject project, MojoExecution execution, Mojo mojo,
>>> >+                                       Throwable cause );
>>> >+}
>>
>> I wonder if it will be easier for future enhancement to use a bean
>> with fields for those objects.
>> MojoExecutionListenerEvent with getMavenSession() etc...
>>
>> Maybe will be simpler to add getter to this bean than changing the
>> signature of the interface.
>> ?
>>
>>
>
> Good idea. Any objections against MojoExecutionEvent and
> ProjectExecutionEvent names?

Sounds good

/Oli
>
> --
> Regards,
> Igor
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and ProjectExecutionListener

Posted by Igor Fedorenko <ig...@ifedorenko.com>.

On 12/16/2013, 5:27, Olivier Lamy wrote:
>> +/**
>> >+ * Extension point that allows build extensions observe and possibly veto mojo executions.
>> >+ *
>> >+ * @see WeakMojoExecutionListener
>> >+ * @since 3.1.2
>> >+ * @provisional This interface is part of work in progress and can be changed or removed without notice.
>> >+ */
>> >+public interface MojoExecutionListener
>> >+{
>> >+    public void beforeMojoExecution( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo )
>> >+        throws MojoExecutionException;
>> >+
>> >+    public void afterMojoExecutionSuccess( MavenSession session, MavenProject project, MojoExecution execution,
>> >+                                           Mojo mojo )
>> >+        throws MojoExecutionException;
>> >+
>> >+    public void afterExecutionFailure( MavenSession session, MavenProject project, MojoExecution execution, Mojo mojo,
>> >+                                       Throwable cause );
>> >+}
> I wonder if it will be easier for future enhancement to use a bean
> with fields for those objects.
> MojoExecutionListenerEvent with getMavenSession() etc...
>
> Maybe will be simpler to add getter to this bean than changing the
> signature of the interface.
> ?
>
>

Good idea. Any objections against MojoExecutionEvent and
ProjectExecutionEvent names?

--
Regards,
Igor

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org