Lucene - Core
  1. Lucene - Core
  2. LUCENE-2867

Change contrib QP API that uses CharSequence as string identifier

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.3
    • Fix Version/s: 3.0.4
    • Component/s: modules/other
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      There are some API methods on contrib queryparser that expects CharSequence as identifier. This is wrong, since it may lead to incorrect or mislead behavior, as shown on LUCENE-2855. To avoid this problem, these APIs will be changed and enforce the use of String instead of CharSequence on version 4. This patch already deprecate the old API methods and add new substitute methods that uses only String.

        Issue Links

          Activity

          Hide
          Adriano Crestani added a comment -

          Hi Robert,

          3.0.x and 2.9.x are bug fix release branches, but in my opinion some of the

          issues surrounding using CharSequence in a HashSet/Map could be considered bugs.

          API changes are not required to fix the CharSequence bug, so I'm not sure if I should be adding and deprecating API methods on 2.9.x and 3.0.x.

          Show
          Adriano Crestani added a comment - Hi Robert, 3.0.x and 2.9.x are bug fix release branches, but in my opinion some of the issues surrounding using CharSequence in a HashSet/Map could be considered bugs. API changes are not required to fix the CharSequence bug, so I'm not sure if I should be adding and deprecating API methods on 2.9.x and 3.0.x.
          Hide
          Robert Muir added a comment -

          Hi Adriano, the 3.x is the dev branch for minor release like 3.1, 3.2, ...

          3.0.x and 2.9.x are bug fix release branches, but in my opinion some of the
          issues surrounding using CharSequence in a HashSet/Map could be considered bugs.

          Show
          Robert Muir added a comment - Hi Adriano, the 3.x is the dev branch for minor release like 3.1, 3.2, ... 3.0.x and 2.9.x are bug fix release branches, but in my opinion some of the issues surrounding using CharSequence in a HashSet/Map could be considered bugs.
          Hide
          Adriano Crestani added a comment -

          Hi Simon,

          Usually we put changes into trunk first and then move them to the branches though.

          Thanks for letting me know about the correct procedure, I will follow it next time. I'm planning to open another JIRA to commit the changes agains the trunk soon.

          Now this issues has been closed / resolved and it has not been committed to trunk and neither to 3.0.4 as stated in the FixVersion? Now I am confused...

          My misunderstanding here, at first, I thought 3x_branch was the next release 3.0.4, I just realized that 3.0.4 is the 3.0 version with bug fixes and 3x_branch is the code for next 3 version (3.1?). So I still need to propagate the API changes to 3.0 branch and trunk (4.0).

          Now I'm confused about what is the main difference between 3.0.x and 3.x. Can any API changes go in 3.0.4 or just bug fixes?

          Thanks,
          Adriano Crestani

          Show
          Adriano Crestani added a comment - Hi Simon, Usually we put changes into trunk first and then move them to the branches though. Thanks for letting me know about the correct procedure, I will follow it next time. I'm planning to open another JIRA to commit the changes agains the trunk soon. Now this issues has been closed / resolved and it has not been committed to trunk and neither to 3.0.4 as stated in the FixVersion? Now I am confused... My misunderstanding here, at first, I thought 3x_branch was the next release 3.0.4, I just realized that 3.0.4 is the 3.0 version with bug fixes and 3x_branch is the code for next 3 version (3.1?). So I still need to propagate the API changes to 3.0 branch and trunk (4.0). Now I'm confused about what is the main difference between 3.0.x and 3.x. Can any API changes go in 3.0.4 or just bug fixes? Thanks, Adriano Crestani
          Hide
          Simon Willnauer added a comment -

          Thanks for reviewing the patch Simon!

          you are welcome... I saw your commit and I wonder why you committed into branches/branch_3x but not in trunk first and then backport? Usually we put changes into trunk first and then move them to the branches though. Now this issues has been closed / resolved and it has not been committed to trunk and neither to 3.0.4 as stated in the FixVersion? Now I am confused...

          So 4.0 changes go into trunk:
          http://svn.apache.org/repos/asf/lucene/dev/trunk/

          Changes for 3.x go here
          http://svn.apache.org/repos/asf/lucene/dev/branches/branch_3x/

          And eventually you deprecations can go here to be in 3.0.4 if that gets released at all...
          http://svn.apache.org/repos/asf/lucene/java/branches/lucene_3_0/

          and / or here:

          http://svn.apache.org/repos/asf/lucene/java/branches/lucene_2_9/

          does that make sense?

          Show
          Simon Willnauer added a comment - Thanks for reviewing the patch Simon! you are welcome... I saw your commit and I wonder why you committed into branches/branch_3x but not in trunk first and then backport? Usually we put changes into trunk first and then move them to the branches though. Now this issues has been closed / resolved and it has not been committed to trunk and neither to 3.0.4 as stated in the FixVersion? Now I am confused... So 4.0 changes go into trunk: http://svn.apache.org/repos/asf/lucene/dev/trunk/ Changes for 3.x go here http://svn.apache.org/repos/asf/lucene/dev/branches/branch_3x/ And eventually you deprecations can go here to be in 3.0.4 if that gets released at all... http://svn.apache.org/repos/asf/lucene/java/branches/lucene_3_0/ and / or here: http://svn.apache.org/repos/asf/lucene/java/branches/lucene_2_9/ does that make sense?
          Hide
          Adriano Crestani added a comment -

          Thanks for reviewing the patch Simon!

          The patch was applied on revision 1059436

          Show
          Adriano Crestani added a comment - Thanks for reviewing the patch Simon! The patch was applied on revision 1059436
          Hide
          Simon Willnauer added a comment -

          Here is the patch that deprecates methods using CharSequence. Can someone please review if I did the API deprecation correctly?

          those comments and annotations look good!

          I was thinking initially that deprecated methods would be removed on version 4, I'm not sure anymore. Will it be removed on 4 or 3.1?

          you should drop those methods from 4.0 code if you want to deprecate. Since this is a contrib you don't necessarily need to deprecate so you could alo drop them from 3.1 or even from 3.0.x

          Show
          Simon Willnauer added a comment - Here is the patch that deprecates methods using CharSequence. Can someone please review if I did the API deprecation correctly? those comments and annotations look good! I was thinking initially that deprecated methods would be removed on version 4, I'm not sure anymore. Will it be removed on 4 or 3.1? you should drop those methods from 4.0 code if you want to deprecate. Since this is a contrib you don't necessarily need to deprecate so you could alo drop them from 3.1 or even from 3.0.x
          Hide
          Adriano Crestani added a comment -

          Here is the patch that deprecates methods using CharSequence. Can someone please review if I did the API deprecation correctly?

          I was thinking initially that deprecated methods would be removed on version 4, I'm not sure anymore. Will it be removed on 4 or 3.1?

          Show
          Adriano Crestani added a comment - Here is the patch that deprecates methods using CharSequence. Can someone please review if I did the API deprecation correctly? I was thinking initially that deprecated methods would be removed on version 4, I'm not sure anymore. Will it be removed on 4 or 3.1?

            People

            • Assignee:
              Adriano Crestani
              Reporter:
              Adriano Crestani
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development