[jexl] 3.1 release review

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jexl] 3.1 release review

Emmanuel Bourg-3
Hi all,

JEXL is ready for a new release, before creating the artifacts and
starting the vote I'd like to ensure there is a consensus on the API
changes. The version 3.1 has 3 incompatible changes that shouldn't have
any impact on the users [1].

I plan to start the vote at the end of the week.

Thank you for your reviews,

Emmanuel Bourg

[1] http://people.apache.org/~ebourg/jexl/site/clirr-report.html

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

garydgregory
Could the breaking changes be documented together in one ticket so we can
review that and provide this ticket as a single point of documentation? An
explanation of why it is OK to break BC would be helpful and reassuring.

2c,
Gary

On Mon, Oct 3, 2016 at 3:05 AM, Emmanuel Bourg <[hidden email]> wrote:

> Hi all,
>
> JEXL is ready for a new release, before creating the artifacts and
> starting the vote I'd like to ensure there is a consensus on the API
> changes. The version 3.1 has 3 incompatible changes that shouldn't have
> any impact on the users [1].
>
> I plan to start the vote at the end of the week.
>
> Thank you for your reviews,
>
> Emmanuel Bourg
>
> [1] http://people.apache.org/~ebourg/jexl/site/clirr-report.html
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


--
E-Mail: [hidden email] | [hidden email]
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

henrib
Resurrecting the thread...Hopefully Emmanuel has a few spare cycles. :-)
I've commented JEXL-220 with the 3 Clirr errors that correspond to adding methods to interfaces that only Jexl is supposed to implement.
I've also added a very clear statements as a waning in the release-notes; it reads as:
{code}

Warning:
========
This release does break compatibility by adding methods to interfaces; these interfaces are not expected to be
implemented by code external to the Jexl project and this compatibility break should remain hypothetical.
It is however possible and we are sorry if it causes problems to any of you.
To mitigate this issue, besides continue using JEXL 3.0, you are likely delegating to JEXL3 objects and you
shall be able to continue doing so with these new methods. The 3 interfaces & methods are:

1 - 'public java.lang.Boolean isCancellable()' in interface org.apache.commons.jexl3.JexlEngine$Options
2 - 'public java.util.concurrent.Callable callable(org.apache.commons.jexl3.JexlContext)'in interface org.apache.commons.jexl3.JexlExpression
3 - 'public java.util.Map getPragmas()'in interface org.apache.commons.jexl3.JxltEngine$Template

{code}

Is this acceptable ?
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

henrib

I've reworded the warning about the source compatibility break in the release notes:
Warning:
========
This release does break source compatibility by adding methods to existing interfaces; these interfaces are however not
expected to be implemented by code external to the Jexl project. This compatibility break should remain hypothetical.
It is nevertheless possible and we are sorry if it causes problems to any of you.
If you encounter this issue, besides continuing using JEXL 3.0, your code likely delegates method calls to JEXL3 objects
and you shall be able to continue doing so with these new methods. The 3 interfaces & methods are:

1 - 'public java.lang.Boolean isCancellable()' in interface org.apache.commons.jexl3.JexlEngine$Options
2 - 'public java.util.concurrent.Callable callable(org.apache.commons.jexl3.JexlContext)'in interface org.apache.commons.jexl3.JexlExpression
3 - 'public java.util.Map getPragmas()'in interface org.apache.commons.jexl3.JxltEngine$Template


If this source compatibility break is not 'permitted' despite its improbability, what option should we take:
- move to another (jexl31) package ?
- add 'extended' interfaces (Options31, JexlExpression31, Template31) ?
- add a utility helper class (Jexl31Helper?) to put the 3 methods as functions ( boolean isCancellable(JExlEngine.Options opt) ?


Comments welcome,
Thanks
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

Matt Sicker
BC can be handled a couple ways here besides renaming:

* Are there already abstract classes that implementations are meant to
extend? If so, that limits the possibility of problems here without using
default methods in Java 8.
* If these are truly public interfaces for users to implement, then you can
extend the interface with an extended version which adds new methods. Try
not to name them Options31 if you can help it.

On 12 March 2017 at 05:16, henrib <[hidden email]> wrote:

>
> I've reworded the warning about the source compatibility break in the
> release notes:
>
>
>
> If this source compatibility break is not 'permitted' despite its
> improbability, what option should we take:
> - move to another (jexl31) package ?
> - add 'extended' interfaces (Options31, JexlExpression31, Template31) ?
> - add a utility helper class (Jexl31Helper?) to put the 3 methods as
> functions ( boolean isCancellable(JExlEngine.Options opt) ?
>
>
> Comments welcome,
> Thanks
>
>
>
> --
> View this message in context: http://apache-commons.680414.
> n4.nabble.com/jexl-3-1-release-review-tp4691513p4696416.html
> Sent from the Commons - Dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


--
Matt Sicker <[hidden email]>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

sebb-2-2
AFAICT two of the interface changes are not needed:

JexlExpression#Callable can be dropped; AFAICT no other changes are needed.
JxltEngine$Template#getPragmas() can be dropped if the test is
modified to use TemplateScript and TS drops the @Override marker

It's probably not easy to drop JexlEngine$Options#isCancellable() from
the interface.
Nor does it look very easy to interpose an Abstract implementation method.
I don't know if one could add an extended Options interface.

Is there a different way to implement isCancellable?
Does it really belong in Options, or could it be added to JexlEngine instead?

Adding methods to an interface does not break binary compatibility,
however AIUI many downstream consumers rely on source builds so
compatibility breaks of any kind should be avoided in the public API.


On 12 March 2017 at 17:09, Matt Sicker <[hidden email]> wrote:

> BC can be handled a couple ways here besides renaming:
>
> * Are there already abstract classes that implementations are meant to
> extend? If so, that limits the possibility of problems here without using
> default methods in Java 8.
> * If these are truly public interfaces for users to implement, then you can
> extend the interface with an extended version which adds new methods. Try
> not to name them Options31 if you can help it.
>
> On 12 March 2017 at 05:16, henrib <[hidden email]> wrote:
>
>>
>> I've reworded the warning about the source compatibility break in the
>> release notes:
>>
>>
>>
>> If this source compatibility break is not 'permitted' despite its
>> improbability, what option should we take:
>> - move to another (jexl31) package ?
>> - add 'extended' interfaces (Options31, JexlExpression31, Template31) ?
>> - add a utility helper class (Jexl31Helper?) to put the 3 methods as
>> functions ( boolean isCancellable(JExlEngine.Options opt) ?
>>
>>
>> Comments welcome,
>> Thanks
>>
>>
>>
>> --
>> View this message in context: http://apache-commons.680414.
>> n4.nabble.com/jexl-3-1-release-review-tp4691513p4696416.html
>> Sent from the Commons - Dev mailing list archive at Nabble.com.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
>
> --
> Matt Sicker <[hidden email]>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

Matt Sicker
It's more of an issue for downstream users who implement the interface as
they'd now have an illegally declared class not implementing all abstract
methods.

On 13 March 2017 at 07:13, sebb <[hidden email]> wrote:

> AFAICT two of the interface changes are not needed:
>
> JexlExpression#Callable can be dropped; AFAICT no other changes are needed.
> JxltEngine$Template#getPragmas() can be dropped if the test is
> modified to use TemplateScript and TS drops the @Override marker
>
> It's probably not easy to drop JexlEngine$Options#isCancellable() from
> the interface.
> Nor does it look very easy to interpose an Abstract implementation method.
> I don't know if one could add an extended Options interface.
>
> Is there a different way to implement isCancellable?
> Does it really belong in Options, or could it be added to JexlEngine
> instead?
>
> Adding methods to an interface does not break binary compatibility,
> however AIUI many downstream consumers rely on source builds so
> compatibility breaks of any kind should be avoided in the public API.
>
>
> On 12 March 2017 at 17:09, Matt Sicker <[hidden email]> wrote:
> > BC can be handled a couple ways here besides renaming:
> >
> > * Are there already abstract classes that implementations are meant to
> > extend? If so, that limits the possibility of problems here without using
> > default methods in Java 8.
> > * If these are truly public interfaces for users to implement, then you
> can
> > extend the interface with an extended version which adds new methods. Try
> > not to name them Options31 if you can help it.
> >
> > On 12 March 2017 at 05:16, henrib <[hidden email]> wrote:
> >
> >>
> >> I've reworded the warning about the source compatibility break in the
> >> release notes:
> >>
> >>
> >>
> >> If this source compatibility break is not 'permitted' despite its
> >> improbability, what option should we take:
> >> - move to another (jexl31) package ?
> >> - add 'extended' interfaces (Options31, JexlExpression31, Template31) ?
> >> - add a utility helper class (Jexl31Helper?) to put the 3 methods as
> >> functions ( boolean isCancellable(JExlEngine.Options opt) ?
> >>
> >>
> >> Comments welcome,
> >> Thanks
> >>
> >>
> >>
> >> --
> >> View this message in context: http://apache-commons.680414.
> >> n4.nabble.com/jexl-3-1-release-review-tp4691513p4696416.html
> >> Sent from the Commons - Dev mailing list archive at Nabble.com.
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >>
> >>
> >
> >
> > --
> > Matt Sicker <[hidden email]>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


--
Matt Sicker <[hidden email]>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

henrib
In reply to this post by sebb-2-2

The interface modifications are fixes to user enhancement requests:

JEXL-211: Add callable method to JexlExpression interface
JEXL-198: JxltEngine Template deos not expose pragmas
JEXL-201: Allow Interpreter to use live values from JexlEngine.Option interface implemented by JexlContext

Note again that these interfaces are *not* expected to be implemented by user code. The likelihood of any user implementing those and not filling enhancements requests or even asking questions in the Apache mailing lists (or Stackoverflow) seems very small...

Choice is please (the few) users using the library or stay true to a rule that protects no real usage in this case. I wish this was seen as an easy choice for the community.
Cheers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

sebb-2-2
On 13 March 2017 at 20:12, henrib <[hidden email]> wrote:

>
> The interface modifications are fixes to user enhancement requests:
>
> JEXL-211: Add callable method to JexlExpression interface
> JEXL-198: JxltEngine Template deos not expose pragmas
> JEXL-201: Allow Interpreter to use live values from JexlEngine.Option
> interface implemented by JexlContext
>
> Note again that these interfaces are *not* expected to be implemented by
> user code.

I'm not sure I understand how that follows.

If the JIRAs are enhancement requests for users, surely the intention
is to allow the users to make use of the new methods?

That suggests that the users are currently using the interfaces.

> The likelihood of any user implementing those and not filling
> enhancements requests or even asking questions in the Apache mailing lists
> (or Stackoverflow) seems very small...

Sorry, no idea what you mean by that.

> Choice is please (the few) users using the library or stay true to a rule
> that protects no real usage in this case. I wish this was seen as an easy
> choice for the community.

Nor that.

> Cheers
>
>
>
> --
> View this message in context: http://apache-commons.680414.n4.nabble.com/jexl-3-1-release-review-tp4691513p4696492.html
> Sent from the Commons - Dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

Matt Sicker
If they're not user-implemented interfaces, then changing them isn't really
a backwards incompatible change.

On 13 March 2017 at 17:50, sebb <[hidden email]> wrote:

> On 13 March 2017 at 20:12, henrib <[hidden email]> wrote:
> >
> > The interface modifications are fixes to user enhancement requests:
> >
> > JEXL-211: Add callable method to JexlExpression interface
> > JEXL-198: JxltEngine Template deos not expose pragmas
> > JEXL-201: Allow Interpreter to use live values from JexlEngine.Option
> > interface implemented by JexlContext
> >
> > Note again that these interfaces are *not* expected to be implemented by
> > user code.
>
> I'm not sure I understand how that follows.
>
> If the JIRAs are enhancement requests for users, surely the intention
> is to allow the users to make use of the new methods?
>
> That suggests that the users are currently using the interfaces.
>
> > The likelihood of any user implementing those and not filling
> > enhancements requests or even asking questions in the Apache mailing
> lists
> > (or Stackoverflow) seems very small...
>
> Sorry, no idea what you mean by that.
>
> > Choice is please (the few) users using the library or stay true to a rule
> > that protects no real usage in this case. I wish this was seen as an easy
> > choice for the community.
>
> Nor that.
>
> > Cheers
> >
> >
> >
> > --
> > View this message in context: http://apache-commons.680414.
> n4.nabble.com/jexl-3-1-release-review-tp4691513p4696492.html
> > Sent from the Commons - Dev mailing list archive at Nabble.com.
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


--
Matt Sicker <[hidden email]>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

sebb-2-2
On 14 March 2017 at 01:38, Matt Sicker <[hidden email]> wrote:
> If they're not user-implemented interfaces, then changing them isn't really
> a backwards incompatible change.

I agree, but since users asked for the changes to the interfaces that
suggests that the interfaces are being used externally.

> On 13 March 2017 at 17:50, sebb <[hidden email]> wrote:
>
>> On 13 March 2017 at 20:12, henrib <[hidden email]> wrote:
>> >
>> > The interface modifications are fixes to user enhancement requests:
>> >
>> > JEXL-211: Add callable method to JexlExpression interface
>> > JEXL-198: JxltEngine Template deos not expose pragmas
>> > JEXL-201: Allow Interpreter to use live values from JexlEngine.Option
>> > interface implemented by JexlContext
>> >
>> > Note again that these interfaces are *not* expected to be implemented by
>> > user code.
>>
>> I'm not sure I understand how that follows.
>>
>> If the JIRAs are enhancement requests for users, surely the intention
>> is to allow the users to make use of the new methods?
>>
>> That suggests that the users are currently using the interfaces.
>>
>> > The likelihood of any user implementing those and not filling
>> > enhancements requests or even asking questions in the Apache mailing
>> lists
>> > (or Stackoverflow) seems very small...
>>
>> Sorry, no idea what you mean by that.
>>
>> > Choice is please (the few) users using the library or stay true to a rule
>> > that protects no real usage in this case. I wish this was seen as an easy
>> > choice for the community.
>>
>> Nor that.
>>
>> > Cheers
>> >
>> >
>> >
>> > --
>> > View this message in context: http://apache-commons.680414.
>> n4.nabble.com/jexl-3-1-release-review-tp4691513p4696492.html
>> > Sent from the Commons - Dev mailing list archive at Nabble.com.
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: [hidden email]
>> > For additional commands, e-mail: [hidden email]
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
>
> --
> Matt Sicker <[hidden email]>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

Matt Sicker
Using interfaces and implementing them are two entirely separate things.
For instance, people use Logger instances, but we've added new APIs to that
interface in log4j-api while maintaining backwards compatibility (though we
do that by providing abstract base classes for implementations to use).

On 13 March 2017 at 21:08, sebb <[hidden email]> wrote:

> On 14 March 2017 at 01:38, Matt Sicker <[hidden email]> wrote:
> > If they're not user-implemented interfaces, then changing them isn't
> really
> > a backwards incompatible change.
>
> I agree, but since users asked for the changes to the interfaces that
> suggests that the interfaces are being used externally.
>
> > On 13 March 2017 at 17:50, sebb <[hidden email]> wrote:
> >
> >> On 13 March 2017 at 20:12, henrib <[hidden email]> wrote:
> >> >
> >> > The interface modifications are fixes to user enhancement requests:
> >> >
> >> > JEXL-211: Add callable method to JexlExpression interface
> >> > JEXL-198: JxltEngine Template deos not expose pragmas
> >> > JEXL-201: Allow Interpreter to use live values from JexlEngine.Option
> >> > interface implemented by JexlContext
> >> >
> >> > Note again that these interfaces are *not* expected to be implemented
> by
> >> > user code.
> >>
> >> I'm not sure I understand how that follows.
> >>
> >> If the JIRAs are enhancement requests for users, surely the intention
> >> is to allow the users to make use of the new methods?
> >>
> >> That suggests that the users are currently using the interfaces.
> >>
> >> > The likelihood of any user implementing those and not filling
> >> > enhancements requests or even asking questions in the Apache mailing
> >> lists
> >> > (or Stackoverflow) seems very small...
> >>
> >> Sorry, no idea what you mean by that.
> >>
> >> > Choice is please (the few) users using the library or stay true to a
> rule
> >> > that protects no real usage in this case. I wish this was seen as an
> easy
> >> > choice for the community.
> >>
> >> Nor that.
> >>
> >> > Cheers
> >> >
> >> >
> >> >
> >> > --
> >> > View this message in context: http://apache-commons.680414.
> >> n4.nabble.com/jexl-3-1-release-review-tp4691513p4696492.html
> >> > Sent from the Commons - Dev mailing list archive at Nabble.com.
> >> >
> >> > ---------------------------------------------------------------------
> >> > To unsubscribe, e-mail: [hidden email]
> >> > For additional commands, e-mail: [hidden email]
> >> >
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >>
> >>
> >
> >
> > --
> > Matt Sicker <[hidden email]>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


--
Matt Sicker <[hidden email]>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [jexl] 3.1 release review

sebb-2-2
On 14 March 2017 at 03:49, Matt Sicker <[hidden email]> wrote:
> Using interfaces and implementing them are two entirely separate things.

Good point; sorry I had overlooked the distinction.

If there are no external implementions (yet) then I agree it should be
OK to release with the existing caveat in the RN.

It would probably be a good idea to warn consumers (in the Javadoc)
that these IFs are not intended for implementation.
Likewise for any other IFs that are intended for use only.

> For instance, people use Logger instances, but we've added new APIs to that
> interface in log4j-api while maintaining backwards compatibility (though we
> do that by providing abstract base classes for implementations to use).

Unfortunately I don't think that can be done here.

> On 13 March 2017 at 21:08, sebb <[hidden email]> wrote:
>
>> On 14 March 2017 at 01:38, Matt Sicker <[hidden email]> wrote:
>> > If they're not user-implemented interfaces, then changing them isn't
>> really
>> > a backwards incompatible change.
>>
>> I agree, but since users asked for the changes to the interfaces that
>> suggests that the interfaces are being used externally.
>>
>> > On 13 March 2017 at 17:50, sebb <[hidden email]> wrote:
>> >
>> >> On 13 March 2017 at 20:12, henrib <[hidden email]> wrote:
>> >> >
>> >> > The interface modifications are fixes to user enhancement requests:
>> >> >
>> >> > JEXL-211: Add callable method to JexlExpression interface
>> >> > JEXL-198: JxltEngine Template deos not expose pragmas
>> >> > JEXL-201: Allow Interpreter to use live values from JexlEngine.Option
>> >> > interface implemented by JexlContext
>> >> >
>> >> > Note again that these interfaces are *not* expected to be implemented
>> by
>> >> > user code.
>> >>
>> >> I'm not sure I understand how that follows.
>> >>
>> >> If the JIRAs are enhancement requests for users, surely the intention
>> >> is to allow the users to make use of the new methods?
>> >>
>> >> That suggests that the users are currently using the interfaces.
>> >>
>> >> > The likelihood of any user implementing those and not filling
>> >> > enhancements requests or even asking questions in the Apache mailing
>> >> lists
>> >> > (or Stackoverflow) seems very small...
>> >>
>> >> Sorry, no idea what you mean by that.
>> >>
>> >> > Choice is please (the few) users using the library or stay true to a
>> rule
>> >> > that protects no real usage in this case. I wish this was seen as an
>> easy
>> >> > choice for the community.
>> >>
>> >> Nor that.
>> >>
>> >> > Cheers
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > View this message in context: http://apache-commons.680414.
>> >> n4.nabble.com/jexl-3-1-release-review-tp4691513p4696492.html
>> >> > Sent from the Commons - Dev mailing list archive at Nabble.com.
>> >> >
>> >> > ---------------------------------------------------------------------
>> >> > To unsubscribe, e-mail: [hidden email]
>> >> > For additional commands, e-mail: [hidden email]
>> >> >
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: [hidden email]
>> >> For additional commands, e-mail: [hidden email]
>> >>
>> >>
>> >
>> >
>> > --
>> > Matt Sicker <[hidden email]>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
>
> --
> Matt Sicker <[hidden email]>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...