Mailing-List: contact dev-help@maven.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Maven Developers List" <dev@maven.apache.org>
Content-Type: text/plain; charset=iso-8859-15; format=flowed; delsp=yes
To: "Maven Developers List" <dev@maven.apache.org>
Subject: Re: [1/2] git commit: MNG-5549 introduced MojoExecutionListener and
 ProjectExecutionListener
References: <cfccba9906b74d3d9a0f6c53070389cb@git.apache.org>
 <CAPoyBqQmtBzHDCYsu78mbmbdpczhK0h0e-FQz9WtGCGMsgYqJQ@mail.gmail.com>
 <52AEF19C.2080201@ifedorenko.com>
 <CAPoyBqSrLJcJ69BWzvsmGyM9Ex_+8MjbjBW=VmnkOQxnjpJ3kg@mail.gmail.com>
 <52AF200A.7000706@ifedorenko.com> <op.w77gxxvokdkhrr@robertscholte>
 <52B2064B.7090804@ifedorenko.com>
Date: Wed, 18 Dec 2013 22:01:15 +0100
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: "Robert Scholte" <rfscholte@apache.org>
Message-ID: <op.w8a3cdsbkdkhrr@robertscholte>
In-Reply-To: <52B2064B.7090804@ifedorenko.com>
User-Agent: Opera Mail/12.16 (Win32)

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  
<igor@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
>> <igor@ifedorenko.com>:
>>
>>>
>>>
>>> On 12/16/2013, 7:39, Olivier Lamy wrote:
>>>> On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <igor@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