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 <i...@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
<i...@ifedorenko.com>:



On 12/16/2013, 7:39, Olivier Lamy wrote:

On Dec 16, 2013 11:27 PM, "Igor Fedorenko" <i...@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-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org


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


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





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

Reply via email to