You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by Itamar Syn-Hershko <it...@code972.com> on 2016/11/03 19:38:48 UTC
Removing LuceneVersion.LUCENE_48 from external API?
Hey folks,
I'm working on some demos, and one things that keeps popping up and to be
frank gets quite annoying is the requirement to specify
LuceneVersion.LUCENE_48 on all public APIs - opening a readers and writers,
analyzers, etc.
Since we only have one version release, and that concept is not going to be
really useful anyway, what do you say we remove (or set a default value for
it) on all public facing APIs?
Cheers,
--
Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko>
Freelance Developer & Consultant
Lucene.NET committer and PMC member
RE: Removing LuceneVersion.LUCENE_48 from external API?
Posted by Shad Storhaug <sh...@shadstorhaug.com>.
Basically, the conclusion I came up with is that this parameter is NOT JUST to support backward compatibility. It is ALSO for supporting forward compatibility.
When the consuming code specifies this parameter, it can be useful for upgrading to the newest version of Lucene to leave the old version compatibility in place. This means you can upgrade Lucene without also being forced to upgrade the index at the same time.
If we took this parameter out of the API, when upgrading Lucene there could be consumers of the API that would either be forced to upgrade their index format immediately OR they would have to find all of the places where they left this parameter out of the code and put it in, specifying the previous version. In other words, there would be no smooth way for consumers who left this parameter out of their code to upgrade to Lucene.Net 5.x or 6.x when it is released.
It seems to me that the Lucene team has thought this through and we should follow their lead by leaving it as a required parameter, to prevent the above scenario from being possible. According to the documentation comment (https://github.com/apache/lucenenet/blob/master/src/Lucene.Net.Core/Util/LuceneVersion.cs#L133-L147), it is this scenario that prompted them to mark the LUCENE_CURRENT parameter obsolete, which is basically the same thing as leaving the value out of the API as far as consuming code is concerned.
That said, we should make our best effort to make this parameter (and any other nullable enum) into a non-nullable in places in the public API where it is nullable. We could, for example, create a "Not Set" value on those enumerations that defaults to 0 (which is the default for any uninitialized value type, making it act very similar to the way it does in Java). Or, in certain cases we might just be able to make the parameter optional, being careful not to affect the upgrade behavior mentioned above.
-----Original Message-----
From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com] On Behalf Of Itamar Syn-Hershko
Sent: Sunday, November 13, 2016 3:18 AM
To: dev@lucenenet.apache.org
Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
I hear you. Would additional constructors that default to Lucene_48 be a better choice then?
--
Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko> Freelance Developer & Consultant Lucene.NET committer and PMC member
On Thu, Nov 10, 2016 at 9:32 AM, Shad Storhaug <sh...@shadstorhaug.com>
wrote:
> I realize that - we wouldn't want to make them optional anyway because
> that would not be CLS compliant (which we may never fully achieve, but
> should probably try to minimize the impact for in case we eventually
> want to go there).
>
> Besides, one of the "users" are the unit tests and they require the
> parameter to be there in order to test the legacy code fully.
> Certainly the parameter will also be needed for those who want to
> upgrade from 3.0.3 without changing their index format right away.
>
> But I agree with you that the majority of the users could get a better
> experience if there were overloads that didn't expect the
> LuceneVersion everywhere.
>
> But then, this really all depends on what the next planned version to
> port is. Imagine everyone switched to this parameterless overload that
> defaulted to 4.8.0, then upgraded to 6.x with the same index - they
> would need to touch every place in their code where the parameter is
> missing and call the overload with 4.8.0 (assuming 6.x goes back at
> least 2 versions of backward compatibility). The worst part is that
> since it is a piece of missing code, it would be impossible to find
> all of those locations with a search, which would make the process
> very manual and prone to error. Maybe it would be better to leave it
> alone...? Certainly, this is the reason why using Version_Current is obsolete.
>
> -----Original Message-----
> From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com]
> On Behalf Of Itamar Syn-Hershko
> Sent: Thursday, November 10, 2016 2:12 PM
> To: dev@lucenenet.apache.org
> Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
>
> Yes, but that essentially means changing them methods signatures since
> it's the first parameter and default arguments need to be last
>
>
> --
>
> Itamar Syn-Hershko
> http://code972.com | @synhershko <https://twitter.com/synhershko>
> Freelance Developer & Consultant Lucene.NET committer and PMC member
>
> On Wed, Nov 9, 2016 at 11:07 PM, Shad Storhaug <sh...@shadstorhaug.com>
> wrote:
>
> > Itamar,
> >
> > I think for those rare cases, we should leave it in. But, it would
> > be a good idea to add overloads that default them to the current
> > version so most users get a streamlined experience.
> >
> > You mentioned that you were "removing" them, I hope that you meant
> > that you are simply providing overloads that don't have them so they
> > are not required.
> >
> > Thanks,
> > Shad Storhaug (NightOwl888)
> >
> > -----Original Message-----
> > From: itamar.synhershko@gmail.com
> > [mailto:itamar.synhershko@gmail.com]
> > On Behalf Of Itamar Syn-Hershko
> > Sent: Thursday, November 10, 2016 10:27 AM
> > To: dev@lucenenet.apache.org
> > Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
> >
> > It's a required argument for those methods - while I think it's too
> > verbose there as well, at least it makes sense because they have
> > many versions. We don't really need it because we only have one
> > version, except from the rare cases backwards supporting indexes
> > that are shared with Java code that maintains them.
> >
> > --
> >
> > Itamar Syn-Hershko
> > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > Freelance Developer & Consultant Lucene.NET committer and PMC member
> >
> > On Mon, Nov 7, 2016 at 6:02 AM, Wyatt Barnett
> > <wy...@gmail.com>
> > wrote:
> >
> > > I think making it an optional parameter sounds like a good idea on
> > > the surface. How does the java library handle this?
> > >
> > > On Thu, Nov 3, 2016 at 3:39 PM Itamar Syn-Hershko
> > > <it...@code972.com>
> > > wrote:
> > >
> > > > Hey folks,
> > > >
> > > > I'm working on some demos, and one things that keeps popping up
> > > > and to be frank gets quite annoying is the requirement to
> > > > specify
> > > > LuceneVersion.LUCENE_48 on all public APIs - opening a readers
> > > > and
> > > writers,
> > > > analyzers, etc.
> > > >
> > > > Since we only have one version release, and that concept is not
> > > > going to
> > > be
> > > > really useful anyway, what do you say we remove (or set a
> > > > default value
> > > for
> > > > it) on all public facing APIs?
> > > >
> > > > Cheers,
> > > >
> > > > --
> > > >
> > > > Itamar Syn-Hershko
> > > > http://code972.com | @synhershko
> > > > <https://twitter.com/synhershko> Freelance Developer &
> > > > Consultant Lucene.NET committer and PMC member
> > > >
> > >
> >
>
Re: Removing LuceneVersion.LUCENE_48 from external API?
Posted by Itamar Syn-Hershko <it...@code972.com>.
I hear you. Would additional constructors that default to Lucene_48 be a
better choice then?
--
Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko>
Freelance Developer & Consultant
Lucene.NET committer and PMC member
On Thu, Nov 10, 2016 at 9:32 AM, Shad Storhaug <sh...@shadstorhaug.com>
wrote:
> I realize that - we wouldn't want to make them optional anyway because
> that would not be CLS compliant (which we may never fully achieve, but
> should probably try to minimize the impact for in case we eventually want
> to go there).
>
> Besides, one of the "users" are the unit tests and they require the
> parameter to be there in order to test the legacy code fully. Certainly the
> parameter will also be needed for those who want to upgrade from 3.0.3
> without changing their index format right away.
>
> But I agree with you that the majority of the users could get a better
> experience if there were overloads that didn't expect the LuceneVersion
> everywhere.
>
> But then, this really all depends on what the next planned version to port
> is. Imagine everyone switched to this parameterless overload that defaulted
> to 4.8.0, then upgraded to 6.x with the same index - they would need to
> touch every place in their code where the parameter is missing and call the
> overload with 4.8.0 (assuming 6.x goes back at least 2 versions of backward
> compatibility). The worst part is that since it is a piece of missing code,
> it would be impossible to find all of those locations with a search, which
> would make the process very manual and prone to error. Maybe it would be
> better to leave it alone...? Certainly, this is the reason why using
> Version_Current is obsolete.
>
> -----Original Message-----
> From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com] On
> Behalf Of Itamar Syn-Hershko
> Sent: Thursday, November 10, 2016 2:12 PM
> To: dev@lucenenet.apache.org
> Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
>
> Yes, but that essentially means changing them methods signatures since
> it's the first parameter and default arguments need to be last
>
>
> --
>
> Itamar Syn-Hershko
> http://code972.com | @synhershko <https://twitter.com/synhershko>
> Freelance Developer & Consultant Lucene.NET committer and PMC member
>
> On Wed, Nov 9, 2016 at 11:07 PM, Shad Storhaug <sh...@shadstorhaug.com>
> wrote:
>
> > Itamar,
> >
> > I think for those rare cases, we should leave it in. But, it would be
> > a good idea to add overloads that default them to the current version
> > so most users get a streamlined experience.
> >
> > You mentioned that you were "removing" them, I hope that you meant
> > that you are simply providing overloads that don't have them so they
> > are not required.
> >
> > Thanks,
> > Shad Storhaug (NightOwl888)
> >
> > -----Original Message-----
> > From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com]
> > On Behalf Of Itamar Syn-Hershko
> > Sent: Thursday, November 10, 2016 10:27 AM
> > To: dev@lucenenet.apache.org
> > Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
> >
> > It's a required argument for those methods - while I think it's too
> > verbose there as well, at least it makes sense because they have many
> > versions. We don't really need it because we only have one version,
> > except from the rare cases backwards supporting indexes that are
> > shared with Java code that maintains them.
> >
> > --
> >
> > Itamar Syn-Hershko
> > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > Freelance Developer & Consultant Lucene.NET committer and PMC member
> >
> > On Mon, Nov 7, 2016 at 6:02 AM, Wyatt Barnett
> > <wy...@gmail.com>
> > wrote:
> >
> > > I think making it an optional parameter sounds like a good idea on
> > > the surface. How does the java library handle this?
> > >
> > > On Thu, Nov 3, 2016 at 3:39 PM Itamar Syn-Hershko
> > > <it...@code972.com>
> > > wrote:
> > >
> > > > Hey folks,
> > > >
> > > > I'm working on some demos, and one things that keeps popping up
> > > > and to be frank gets quite annoying is the requirement to specify
> > > > LuceneVersion.LUCENE_48 on all public APIs - opening a readers and
> > > writers,
> > > > analyzers, etc.
> > > >
> > > > Since we only have one version release, and that concept is not
> > > > going to
> > > be
> > > > really useful anyway, what do you say we remove (or set a default
> > > > value
> > > for
> > > > it) on all public facing APIs?
> > > >
> > > > Cheers,
> > > >
> > > > --
> > > >
> > > > Itamar Syn-Hershko
> > > > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > > > Freelance Developer & Consultant Lucene.NET committer and PMC
> > > > member
> > > >
> > >
> >
>
RE: Removing LuceneVersion.LUCENE_48 from external API?
Posted by Shad Storhaug <sh...@shadstorhaug.com>.
I realize that - we wouldn't want to make them optional anyway because that would not be CLS compliant (which we may never fully achieve, but should probably try to minimize the impact for in case we eventually want to go there).
Besides, one of the "users" are the unit tests and they require the parameter to be there in order to test the legacy code fully. Certainly the parameter will also be needed for those who want to upgrade from 3.0.3 without changing their index format right away.
But I agree with you that the majority of the users could get a better experience if there were overloads that didn't expect the LuceneVersion everywhere.
But then, this really all depends on what the next planned version to port is. Imagine everyone switched to this parameterless overload that defaulted to 4.8.0, then upgraded to 6.x with the same index - they would need to touch every place in their code where the parameter is missing and call the overload with 4.8.0 (assuming 6.x goes back at least 2 versions of backward compatibility). The worst part is that since it is a piece of missing code, it would be impossible to find all of those locations with a search, which would make the process very manual and prone to error. Maybe it would be better to leave it alone...? Certainly, this is the reason why using Version_Current is obsolete.
-----Original Message-----
From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com] On Behalf Of Itamar Syn-Hershko
Sent: Thursday, November 10, 2016 2:12 PM
To: dev@lucenenet.apache.org
Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
Yes, but that essentially means changing them methods signatures since it's the first parameter and default arguments need to be last
--
Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko> Freelance Developer & Consultant Lucene.NET committer and PMC member
On Wed, Nov 9, 2016 at 11:07 PM, Shad Storhaug <sh...@shadstorhaug.com>
wrote:
> Itamar,
>
> I think for those rare cases, we should leave it in. But, it would be
> a good idea to add overloads that default them to the current version
> so most users get a streamlined experience.
>
> You mentioned that you were "removing" them, I hope that you meant
> that you are simply providing overloads that don't have them so they
> are not required.
>
> Thanks,
> Shad Storhaug (NightOwl888)
>
> -----Original Message-----
> From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com]
> On Behalf Of Itamar Syn-Hershko
> Sent: Thursday, November 10, 2016 10:27 AM
> To: dev@lucenenet.apache.org
> Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
>
> It's a required argument for those methods - while I think it's too
> verbose there as well, at least it makes sense because they have many
> versions. We don't really need it because we only have one version,
> except from the rare cases backwards supporting indexes that are
> shared with Java code that maintains them.
>
> --
>
> Itamar Syn-Hershko
> http://code972.com | @synhershko <https://twitter.com/synhershko>
> Freelance Developer & Consultant Lucene.NET committer and PMC member
>
> On Mon, Nov 7, 2016 at 6:02 AM, Wyatt Barnett
> <wy...@gmail.com>
> wrote:
>
> > I think making it an optional parameter sounds like a good idea on
> > the surface. How does the java library handle this?
> >
> > On Thu, Nov 3, 2016 at 3:39 PM Itamar Syn-Hershko
> > <it...@code972.com>
> > wrote:
> >
> > > Hey folks,
> > >
> > > I'm working on some demos, and one things that keeps popping up
> > > and to be frank gets quite annoying is the requirement to specify
> > > LuceneVersion.LUCENE_48 on all public APIs - opening a readers and
> > writers,
> > > analyzers, etc.
> > >
> > > Since we only have one version release, and that concept is not
> > > going to
> > be
> > > really useful anyway, what do you say we remove (or set a default
> > > value
> > for
> > > it) on all public facing APIs?
> > >
> > > Cheers,
> > >
> > > --
> > >
> > > Itamar Syn-Hershko
> > > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > > Freelance Developer & Consultant Lucene.NET committer and PMC
> > > member
> > >
> >
>
Re: Removing LuceneVersion.LUCENE_48 from external API?
Posted by Itamar Syn-Hershko <it...@code972.com>.
Yes, but that essentially means changing them methods signatures since it's
the first parameter and default arguments need to be last
--
Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko>
Freelance Developer & Consultant
Lucene.NET committer and PMC member
On Wed, Nov 9, 2016 at 11:07 PM, Shad Storhaug <sh...@shadstorhaug.com>
wrote:
> Itamar,
>
> I think for those rare cases, we should leave it in. But, it would be a
> good idea to add overloads that default them to the current version so most
> users get a streamlined experience.
>
> You mentioned that you were "removing" them, I hope that you meant that
> you are simply providing overloads that don't have them so they are not
> required.
>
> Thanks,
> Shad Storhaug (NightOwl888)
>
> -----Original Message-----
> From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com] On
> Behalf Of Itamar Syn-Hershko
> Sent: Thursday, November 10, 2016 10:27 AM
> To: dev@lucenenet.apache.org
> Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
>
> It's a required argument for those methods - while I think it's too
> verbose there as well, at least it makes sense because they have many
> versions. We don't really need it because we only have one version, except
> from the rare cases backwards supporting indexes that are shared with Java
> code that maintains them.
>
> --
>
> Itamar Syn-Hershko
> http://code972.com | @synhershko <https://twitter.com/synhershko>
> Freelance Developer & Consultant Lucene.NET committer and PMC member
>
> On Mon, Nov 7, 2016 at 6:02 AM, Wyatt Barnett <wy...@gmail.com>
> wrote:
>
> > I think making it an optional parameter sounds like a good idea on the
> > surface. How does the java library handle this?
> >
> > On Thu, Nov 3, 2016 at 3:39 PM Itamar Syn-Hershko <it...@code972.com>
> > wrote:
> >
> > > Hey folks,
> > >
> > > I'm working on some demos, and one things that keeps popping up and
> > > to be frank gets quite annoying is the requirement to specify
> > > LuceneVersion.LUCENE_48 on all public APIs - opening a readers and
> > writers,
> > > analyzers, etc.
> > >
> > > Since we only have one version release, and that concept is not
> > > going to
> > be
> > > really useful anyway, what do you say we remove (or set a default
> > > value
> > for
> > > it) on all public facing APIs?
> > >
> > > Cheers,
> > >
> > > --
> > >
> > > Itamar Syn-Hershko
> > > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > > Freelance Developer & Consultant Lucene.NET committer and PMC member
> > >
> >
>
RE: Removing LuceneVersion.LUCENE_48 from external API?
Posted by Shad Storhaug <sh...@shadstorhaug.com>.
Itamar,
I think for those rare cases, we should leave it in. But, it would be a good idea to add overloads that default them to the current version so most users get a streamlined experience.
You mentioned that you were "removing" them, I hope that you meant that you are simply providing overloads that don't have them so they are not required.
Thanks,
Shad Storhaug (NightOwl888)
-----Original Message-----
From: itamar.synhershko@gmail.com [mailto:itamar.synhershko@gmail.com] On Behalf Of Itamar Syn-Hershko
Sent: Thursday, November 10, 2016 10:27 AM
To: dev@lucenenet.apache.org
Subject: Re: Removing LuceneVersion.LUCENE_48 from external API?
It's a required argument for those methods - while I think it's too verbose there as well, at least it makes sense because they have many versions. We don't really need it because we only have one version, except from the rare cases backwards supporting indexes that are shared with Java code that maintains them.
--
Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko> Freelance Developer & Consultant Lucene.NET committer and PMC member
On Mon, Nov 7, 2016 at 6:02 AM, Wyatt Barnett <wy...@gmail.com>
wrote:
> I think making it an optional parameter sounds like a good idea on the
> surface. How does the java library handle this?
>
> On Thu, Nov 3, 2016 at 3:39 PM Itamar Syn-Hershko <it...@code972.com>
> wrote:
>
> > Hey folks,
> >
> > I'm working on some demos, and one things that keeps popping up and
> > to be frank gets quite annoying is the requirement to specify
> > LuceneVersion.LUCENE_48 on all public APIs - opening a readers and
> writers,
> > analyzers, etc.
> >
> > Since we only have one version release, and that concept is not
> > going to
> be
> > really useful anyway, what do you say we remove (or set a default
> > value
> for
> > it) on all public facing APIs?
> >
> > Cheers,
> >
> > --
> >
> > Itamar Syn-Hershko
> > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > Freelance Developer & Consultant Lucene.NET committer and PMC member
> >
>
Re: Removing LuceneVersion.LUCENE_48 from external API?
Posted by Itamar Syn-Hershko <it...@code972.com>.
It's a required argument for those methods - while I think it's too verbose
there as well, at least it makes sense because they have many versions. We
don't really need it because we only have one version, except from the rare
cases backwards supporting indexes that are shared with Java code that
maintains them.
--
Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko>
Freelance Developer & Consultant
Lucene.NET committer and PMC member
On Mon, Nov 7, 2016 at 6:02 AM, Wyatt Barnett <wy...@gmail.com>
wrote:
> I think making it an optional parameter sounds like a good idea on the
> surface. How does the java library handle this?
>
> On Thu, Nov 3, 2016 at 3:39 PM Itamar Syn-Hershko <it...@code972.com>
> wrote:
>
> > Hey folks,
> >
> > I'm working on some demos, and one things that keeps popping up and to be
> > frank gets quite annoying is the requirement to specify
> > LuceneVersion.LUCENE_48 on all public APIs - opening a readers and
> writers,
> > analyzers, etc.
> >
> > Since we only have one version release, and that concept is not going to
> be
> > really useful anyway, what do you say we remove (or set a default value
> for
> > it) on all public facing APIs?
> >
> > Cheers,
> >
> > --
> >
> > Itamar Syn-Hershko
> > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > Freelance Developer & Consultant
> > Lucene.NET committer and PMC member
> >
>
Re: Removing LuceneVersion.LUCENE_48 from external API?
Posted by Wyatt Barnett <wy...@gmail.com>.
I think making it an optional parameter sounds like a good idea on the
surface. How does the java library handle this?
On Thu, Nov 3, 2016 at 3:39 PM Itamar Syn-Hershko <it...@code972.com>
wrote:
> Hey folks,
>
> I'm working on some demos, and one things that keeps popping up and to be
> frank gets quite annoying is the requirement to specify
> LuceneVersion.LUCENE_48 on all public APIs - opening a readers and writers,
> analyzers, etc.
>
> Since we only have one version release, and that concept is not going to be
> really useful anyway, what do you say we remove (or set a default value for
> it) on all public facing APIs?
>
> Cheers,
>
> --
>
> Itamar Syn-Hershko
> http://code972.com | @synhershko <https://twitter.com/synhershko>
> Freelance Developer & Consultant
> Lucene.NET committer and PMC member
>