[
https://issues.apache.org/jira/browse/LUCENE-5859?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14080133#comment-14080133
]
Hoss Man commented on LUCENE-5859:
----------------------------------
bq. Commit 1614778 from Robert Muir in branch 'dev/trunk'
Please revert this until there can be more discussion.
bq. And you will notice all the backcompat is preserved: again no runtime
behavior changed here. I didnt change the back compat.
This commit may not change the backcompat behavior of any existing class, but
you hobbled our ability to make _future_ changes w/o jumping through hoops to
avoid breaking back compat.
You seem to be conflating a general API convention that _allows_ for variations
in behavior with the idea that if every class following this convention doesn't
already take advantage of this behavior then it's "dead code" that is in some
way broken.
bq. ThaiAnalyzer, TurkishAnalyzer, NGrams, unfortunately they all still have
this silly parameter, because they actually make use of it, for back compat
purposes since they had serious changes in 4.x.
It boggles my mind how you can point out that these classes remain unchanged,
and still call the param "silly" while you gut it from all other classes
baffles me.
Let's look at TurkishAnalyzer for example -- it uses the matchVersion param to
decide whether or not to include "ApostropheFilter" in the stream, because in
4.8, Ahmet suggested adding the ApostropheFilter and you thought it was a great
idea and should be part of the default behavior of TurkishAnalyzer.
The fact that TurkishAnalyzer already had a constructor that took in a Version
param is the reason we could make this change to the default behavior of the
analyzer, and improve the out of the box experience for all future users of
the class, w/o breaking any back compact for existing users.
if you had committed r1614778 prior to LUCENE-5482 being opened, and included
TurkishAnalyzer in the list of classes you purged Version from, we would not
have been able to change the _default_ behavior of TurkishAnalyzer w/o breaking
backcompat for existing users. This wasn't about a "bug fix" it was about an
improved default user experience moving forward, w/o breaking things for
existing users who had working systems
What you have done now is put is in the position that for any similar
improvements that anyone ever suggests to any analyzer (except ThaiAnalyzer and
TurkishAnalyzer which you spared) we either can't change the default behavior,
or we have to break backcompat for existing users.
(you may argue that this is only a trunk change, but that just kicks the can
down the road -- w/o these constructors in place when 5.0 is released, we'll
face all the same problems in the 5.x releases that the existence of Version
has helped us avoid in the 4.x release)
----
* Your initial suggestion of adding non-Version constructors for people who
don't care about backcompat was a good one.
* your r1614698 commit to cleanup and simplify 99.9% of the calls to
newIndexWriterConfig from tests was also a great simplification
* r1614778 is a terrible, crippling, blow to our ability to make changes in the
future w/o breaking back compat to existing users.
> Remove Version.java completely
> ------------------------------
>
> Key: LUCENE-5859
> URL: https://issues.apache.org/jira/browse/LUCENE-5859
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Robert Muir
> Fix For: 5.0
>
> Attachments: LUCENE-5859_dead_code.patch
>
>
> This has always been a mess: analyzers are easy enough to make on your own,
> we don't need to "take responsibility" for the users analysis chain for 2
> major releases.
> The code maintenance is horrible here.
> This creates a huge usability issue too, and as seen from numerous mailing
> list issues, users don't even understand how this versioning works anyway.
> I'm sure someone will whine if i try to remove these constants, but we can at
> least make no-arg ctors forwarding to VERSION_CURRENT so that people who
> don't care about back compat (e.g. just prototyping) don't have to deal with
> the horribly complex versioning system.
> If you want to make the argument that doing this is "trappy" (i heard this
> before), i think thats bogus, and ill counter by trying to remove them.
> Either way, I'm personally not going to add any of this kind of back compat
> logic myself ever again.
> Updated: description of the issue updated as expected. We should remove this
> API completely. No one else on the planet has APIs that require a mandatory
> version parameter.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
