Lucene - Core
  1. Lucene - Core
  2. LUCENE-5449

Two ancient classes renamed to be less peculiar: _TestHelper and _TestUtil

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.6.1
    • Fix Version/s: 4.8, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      _TestUtil and _TestHelper begin with _ for historical reasons that don't apply any longer. Lets eliminate those _'s.

        Activity

        Hide
        Uwe Schindler added a comment -

        +1 to fix that. We should also use static methods from those classes via static imports for better readability.

        Show
        Uwe Schindler added a comment - +1 to fix that. We should also use static methods from those classes via static imports for better readability.
        Hide
        ASF GitHub Bot added a comment -

        GitHub user benson-basis opened a pull request:

        https://github.com/apache/lucene-solr/pull/36

        LUCENE-5449: Rename _TestUtil to TestUtil.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/benson-basis/lucene-solr LUCENE-5449-rename-test-classes

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/lucene-solr/pull/36.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #36


        commit 8d62921596eb4e2799805582907b3bac546878b2
        Author: Benson Margulies <benson@basistech.com>
        Date: 2014-02-18T03:13:39Z

        LUCENE-5449: Rename _TestUtil to TestUtil.


        Show
        ASF GitHub Bot added a comment - GitHub user benson-basis opened a pull request: https://github.com/apache/lucene-solr/pull/36 LUCENE-5449 : Rename _TestUtil to TestUtil. You can merge this pull request into a Git repository by running: $ git pull https://github.com/benson-basis/lucene-solr LUCENE-5449 -rename-test-classes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/36.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #36 commit 8d62921596eb4e2799805582907b3bac546878b2 Author: Benson Margulies <benson@basistech.com> Date: 2014-02-18T03:13:39Z LUCENE-5449 : Rename _TestUtil to TestUtil.
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        Benson Margulies added a comment -

        Uwe Schindler, I am not enthusiastic about > 1000 edits to change from importing the class to static importing the methods. Do you see this as a requirement, or just a desirable practice going forward?

        Show
        Benson Margulies added a comment - Uwe Schindler , I am not enthusiastic about > 1000 edits to change from importing the class to static importing the methods. Do you see this as a requirement, or just a desirable practice going forward?
        Hide
        Uwe Schindler added a comment -

        Hi Beson, my comment was for the future. I would suggest to use a ststic import for new tests to make code cleaner. Now doing a convert from "import of class + Classname.method()" to "static import of class.method + method()" is not really useful.

        In general for "utility" classes, I think using static imports should be used (if there are no other conflicts). But that's just my opinion.

        Show
        Uwe Schindler added a comment - Hi Beson, my comment was for the future . I would suggest to use a ststic import for new tests to make code cleaner. Now doing a convert from "import of class + Classname.method()" to "static import of class.method + method()" is not really useful. In general for "utility" classes, I think using static imports should be used (if there are no other conflicts). But that's just my opinion.
        Hide
        Michael McCandless added a comment -

        I think even if we switch to static imports, we should do it separately?

        When making massive automated changes I think it's best to do each one separately (and, also separately from any manual edits)...

        Show
        Michael McCandless added a comment - I think even if we switch to static imports, we should do it separately? When making massive automated changes I think it's best to do each one separately (and, also separately from any manual edits)...
        Hide
        Benson Margulies added a comment -

        OK, then this is good to go. (I did include one example of switching to a static import, even though I agree with Michael McCandless in general.

        Show
        Benson Margulies added a comment - OK, then this is good to go. (I did include one example of switching to a static import, even though I agree with Michael McCandless in general.
        Hide
        Uwe Schindler added a comment -

        +1 to commit. I hope you get the file rename managed (see Shai Erera's mail on dev list).

        Show
        Uwe Schindler added a comment - +1 to commit. I hope you get the file rename managed (see Shai Erera 's mail on dev list).
        Hide
        ASF subversion and git services added a comment -

        Commit 1569597 from Benson Margulies in branch 'dev/trunk'
        [ https://svn.apache.org/r1569597 ]

        LUCENE-5449: Rename _TestUtil to TestUtil.

        Show
        ASF subversion and git services added a comment - Commit 1569597 from Benson Margulies in branch 'dev/trunk' [ https://svn.apache.org/r1569597 ] LUCENE-5449 : Rename _TestUtil to TestUtil.
        Hide
        ASF subversion and git services added a comment -

        Commit 1569598 from Benson Margulies in branch 'dev/trunk'
        [ https://svn.apache.org/r1569598 ]

        LUCENE-5449: _TestHelper -> TestHelper.

        Show
        ASF subversion and git services added a comment - Commit 1569598 from Benson Margulies in branch 'dev/trunk' [ https://svn.apache.org/r1569598 ] LUCENE-5449 : _TestHelper -> TestHelper.
        Hide
        ASF subversion and git services added a comment -

        Commit 1569599 from Benson Margulies in branch 'dev/trunk'
        [ https://svn.apache.org/r1569599 ]

        LUCENE-5449: add CHANGES.txt.

        This closes #36.

        Show
        ASF subversion and git services added a comment - Commit 1569599 from Benson Margulies in branch 'dev/trunk' [ https://svn.apache.org/r1569599 ] LUCENE-5449 : add CHANGES.txt. This closes #36.
        Hide
        ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/lucene-solr/pull/36

        Show
        ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/lucene-solr/pull/36
        Hide
        ASF subversion and git services added a comment -

        Commit 1569739 from Benson Margulies in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1569739 ]

        LUCENE-5449: backport to 4.x, 1 of 2.

        Show
        ASF subversion and git services added a comment - Commit 1569739 from Benson Margulies in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1569739 ] LUCENE-5449 : backport to 4.x, 1 of 2.
        Hide
        ASF subversion and git services added a comment -

        Commit 1569747 from Robert Muir in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1569747 ]

        LUCENE-5449: quick find-replace to unbreak the build (see my mail to list)

        Show
        ASF subversion and git services added a comment - Commit 1569747 from Robert Muir in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1569747 ] LUCENE-5449 : quick find-replace to unbreak the build (see my mail to list)
        Hide
        Benson Margulies added a comment - - edited

        I'm unable to reconstruct how I laid this egg. My only theory is that I had somehow cd'd back to the wrong tree before running ant precommit after thinking I had made all the corrections after the merge. Rob's commit really just finishes my work on 'part 1': part 2 was always going to be the _TestHelper commit. Let's see if I can get that one right.

        Show
        Benson Margulies added a comment - - edited I'm unable to reconstruct how I laid this egg. My only theory is that I had somehow cd'd back to the wrong tree before running ant precommit after thinking I had made all the corrections after the merge. Rob's commit really just finishes my work on 'part 1': part 2 was always going to be the _TestHelper commit. Let's see if I can get that one right.
        Hide
        ASF subversion and git services added a comment -

        Commit 1569973 from Benson Margulies in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1569973 ]

        LUCENE-5449: backport the _TestHelper changes. if this one
        breaks the build I'm switching to agriculture.

        Show
        ASF subversion and git services added a comment - Commit 1569973 from Benson Margulies in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1569973 ] LUCENE-5449 : backport the _TestHelper changes. if this one breaks the build I'm switching to agriculture.
        Hide
        ASF subversion and git services added a comment -

        Commit 1569975 from Benson Margulies in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1569975 ]

        LUCENE-5449: patch up CHANGES.txt(s) to reflect backports.

        Show
        ASF subversion and git services added a comment - Commit 1569975 from Benson Margulies in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1569975 ] LUCENE-5449 : patch up CHANGES.txt(s) to reflect backports.
        Hide
        Robert Muir added a comment -

        Thanks for this cleanup effort!

        Show
        Robert Muir added a comment - Thanks for this cleanup effort!
        Hide
        Uwe Schindler added a comment -

        Close issue after release of 4.8.0

        Show
        Uwe Schindler added a comment - Close issue after release of 4.8.0

          People

          • Assignee:
            Benson Margulies
            Reporter:
            Benson Margulies
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development