Lucene - Core
  1. Lucene - Core
  2. LUCENE-5632

transition Version constants from LUCENE_MN to LUCENE_M_N

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.9, 6.0
    • Component/s: core/other
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      We should fix this, otherwise the constants will be hard to read (e.g. Version.LUCENE_410, is it 4.1.0 or 4.10 or whatever).

      I do not want this to be an excuse for an arbitrary 5.0 release that does not have the features expected of a major release

      1. LUCENE-5632.patch
        56 kB
        Uwe Schindler
      2. LUCENE-5632.patch
        33 kB
        Uwe Schindler
      3. LUCENE-5632.patch
        32 kB
        Uwe Schindler
      4. LUCENE-5632-4x.patch
        134 kB
        Uwe Schindler

        Activity

        Hide
        Uwe Schindler added a comment - - edited

        One idea. We redfine the enum with the new syntax. In Lucene 4.x we may additionally define the old constants as additional alias statics (+ deprecated) inside the enum? Java does not allow additional constants in an enum that are identical to others, but we can maybe move the deprecated ones between the real ones (like: LUCENE_4_0, @Deprecated LUCENE_40, LUCENE_4_1, @Deprecated LUCENE_41, although I am not sure if they should come before or after, but we can add magic to the version comparison functions: onOrAfter() can accept both).
        We must also fix the parseVersionLenient for use by Solr + ElasticSearch.

        Show
        Uwe Schindler added a comment - - edited One idea. We redfine the enum with the new syntax. In Lucene 4.x we may additionally define the old constants as additional alias statics (+ deprecated) inside the enum? Java does not allow additional constants in an enum that are identical to others, but we can maybe move the deprecated ones between the real ones (like: LUCENE_4_0, @Deprecated LUCENE_40, LUCENE_4_1, @Deprecated LUCENE_41 , although I am not sure if they should come before or after, but we can add magic to the version comparison functions: onOrAfter() can accept both). We must also fix the parseVersionLenient for use by Solr + ElasticSearch.
        Hide
        Uwe Schindler added a comment -

        In fact it is possible to add "deprecated" old constants somewhere at the end of the enum. Those are no real enum constants (they dont work in switch statements), but for the general use case of matchVersion parameters, this is fine:

        @Deprecated
        public static final Version LUCENE_41 = LUCENE_4_1;
        
        Show
        Uwe Schindler added a comment - In fact it is possible to add "deprecated" old constants somewhere at the end of the enum. Those are no real enum constants (they dont work in switch statements), but for the general use case of matchVersion parameters, this is fine: @Deprecated public static final Version LUCENE_41 = LUCENE_4_1;
        Hide
        Uwe Schindler added a comment -

        Patch, interestingly not so many things changed.

        I did not yet run tests, but I also fixed the parser.

        Show
        Uwe Schindler added a comment - Patch, interestingly not so many things changed. I did not yet run tests, but I also fixed the parser.
        Hide
        Uwe Schindler added a comment -

        New patch, which passes all tests (unmodified). Solr tests also pass with unmodified config files.

        In fact, in branch_4x there are more occurences, but the whole patch is more or less a Eclipse refactor rename. So I would suggest to fix this for 4.x in the way like the attached patch and remove in trunk all deprecated constants completely (so simply do rename in trunk).

        Show
        Uwe Schindler added a comment - New patch, which passes all tests (unmodified). Solr tests also pass with unmodified config files. In fact, in branch_4x there are more occurences, but the whole patch is more or less a Eclipse refactor rename. So I would suggest to fix this for 4.x in the way like the attached patch and remove in trunk all deprecated constants completely (so simply do rename in trunk).
        Hide
        Hoss Man added a comment -

        So I would suggest to fix this for 4.x in the way like the attached patch and remove in trunk all deprecated constants completely (so simply do rename in trunk).

        I think it would probably be best to keep the (new) parseLeniently on trunk as well though (not just on 4x) so that strings like "LUCENE_47" continue to work on trunk.

        Show
        Hoss Man added a comment - So I would suggest to fix this for 4.x in the way like the attached patch and remove in trunk all deprecated constants completely (so simply do rename in trunk). I think it would probably be best to keep the (new) parseLeniently on trunk as well though (not just on 4x) so that strings like "LUCENE_47" continue to work on trunk.
        Hide
        Uwe Schindler added a comment -

        Thanks Hoss!
        I will keep the lenient parser for 5.0, too. I will only remove the constants in source code, so Java code can no longer use them.
        In fact, I will start on the 4.x branch and clean it up including all the deprecations. I will then merge to trunk. This is the better approach for this task.

        Show
        Uwe Schindler added a comment - Thanks Hoss! I will keep the lenient parser for 5.0, too. I will only remove the constants in source code, so Java code can no longer use them. In fact, I will start on the 4.x branch and clean it up including all the deprecations. I will then merge to trunk. This is the better approach for this task.
        Hide
        Uwe Schindler added a comment -

        Patch for 4.x, I will commit this once tests are happy.

        Show
        Uwe Schindler added a comment - Patch for 4.x, I will commit this once tests are happy.
        Hide
        Uwe Schindler added a comment -

        Tests are happy, committing and forward-porting!

        Show
        Uwe Schindler added a comment - Tests are happy, committing and forward-porting!
        Hide
        ASF subversion and git services added a comment -

        Commit 1591333 from Uwe Schindler in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1591333 ]

        LUCENE-5632: Transition Version constants from LUCENE_MN to LUCENE_M_N

        Show
        ASF subversion and git services added a comment - Commit 1591333 from Uwe Schindler in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1591333 ] LUCENE-5632 : Transition Version constants from LUCENE_MN to LUCENE_M_N
        Hide
        Uwe Schindler added a comment -

        Merged patch for 5.0 (trunk). Will commit after tests are happy.

        Show
        Uwe Schindler added a comment - Merged patch for 5.0 (trunk). Will commit after tests are happy.
        Hide
        ASF subversion and git services added a comment -

        Commit 1591365 from Uwe Schindler in branch 'dev/trunk'
        [ https://svn.apache.org/r1591365 ]

        Merged revision(s) 1591333 from lucene/dev/branches/branch_4x:
        LUCENE-5632: Transition Version constants from LUCENE_MN to LUCENE_M_N

        Show
        ASF subversion and git services added a comment - Commit 1591365 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1591365 ] Merged revision(s) 1591333 from lucene/dev/branches/branch_4x: LUCENE-5632 : Transition Version constants from LUCENE_MN to LUCENE_M_N

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development