Lucene - Core
  1. Lucene - Core
  2. LUCENE-5512

Remove redundant typing (diamond operator) in trunk

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.8, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New
    1. LUCENE-5512.patch
      1.90 MB
      Furkan KAMACI
    2. LUCENE-5512.patch
      955 kB
      Furkan KAMACI
    3. LUCENE-5512.patch
      478 kB
      Robert Muir

      Activity

      Hide
      Furkan KAMACI added a comment -

      Currently I've found 1542 usage for it at trunk. I can work for this issue.

      Show
      Furkan KAMACI added a comment - Currently I've found 1542 usage for it at trunk. I can work for this issue.
      Hide
      Robert Muir added a comment -

      There are way more than that. I don't recommend the use of automated tools (it sounds easy, but it doesnt take care of style, generated code, etc).

      Show
      Robert Muir added a comment - There are way more than that. I don't recommend the use of automated tools (it sounds easy, but it doesnt take care of style, generated code, etc).
      Hide
      Furkan KAMACI added a comment -

      I'll not use an automated tool because of it is an important thing so we should be careful.

      Show
      Furkan KAMACI added a comment - I'll not use an automated tool because of it is an important thing so we should be careful.
      Hide
      Erick Erickson added a comment -

      Sure hope the eventual (massive) check-in/merge works. Is there any merit in doing this in chunks that are more bite-sized? Perhaps making this an umbrella JIRA?

      I just worry that this is going to touch lots and lots and lots of files in the code base, inconveniencing people who are in the middle of some work.

      And I have to ask, what good is this doing us? Does it make any functional difference or is this simply esthetic? If the latter, then I suspect that doing this is going to cause some disruption to no good purpose. Reconciling any update issues for people who have significant outstanding chunks of code with changes may be "interesting".

      Or I may be imagining problems that don't actually exist. I guess under any circumstances since I'm not doing the work I don't really have much say...

      Show
      Erick Erickson added a comment - Sure hope the eventual (massive) check-in/merge works. Is there any merit in doing this in chunks that are more bite-sized? Perhaps making this an umbrella JIRA? I just worry that this is going to touch lots and lots and lots of files in the code base, inconveniencing people who are in the middle of some work. And I have to ask, what good is this doing us? Does it make any functional difference or is this simply esthetic? If the latter, then I suspect that doing this is going to cause some disruption to no good purpose. Reconciling any update issues for people who have significant outstanding chunks of code with changes may be "interesting". Or I may be imagining problems that don't actually exist. I guess under any circumstances since I'm not doing the work I don't really have much say...
      Hide
      Robert Muir added a comment -

      Furkan: i'll give you my patch if you want to take over?

      The safest approach: make it a compile error in eclipse.

      Show
      Robert Muir added a comment - Furkan: i'll give you my patch if you want to take over? The safest approach: make it a compile error in eclipse.
      Hide
      Robert Muir added a comment -

      Furkan: attached is my patch, i did some parts of the codebase. Happy to have you take over here!

      Show
      Robert Muir added a comment - Furkan: attached is my patch, i did some parts of the codebase. Happy to have you take over here!
      Hide
      Uwe Schindler added a comment -

      I think before backporting to 4.x, I would do the merge of the previous patches. Once the vote is over, I will start and backport as many as possible of the previous commits for Java 7. This includes reverting the "quick fix" commits to prevent compile issues in 4.x.

      My personal opinion about the diamond operator is mixed: I don't see this as important. Much more important is migrating over the code to try-with resources and only use IOUtils at places where the open/close is not in the same code block. But this needs more careful review!

      Show
      Uwe Schindler added a comment - I think before backporting to 4.x, I would do the merge of the previous patches. Once the vote is over, I will start and backport as many as possible of the previous commits for Java 7. This includes reverting the "quick fix" commits to prevent compile issues in 4.x. My personal opinion about the diamond operator is mixed: I don't see this as important. Much more important is migrating over the code to try-with resources and only use IOUtils at places where the open/close is not in the same code block. But this needs more careful review!
      Hide
      Robert Muir added a comment -

      But we don't need to wait on anything to clean up trunk. Its been on java7 for a long time.

      Show
      Robert Muir added a comment - But we don't need to wait on anything to clean up trunk. Its been on java7 for a long time.
      Hide
      Uwe Schindler added a comment -

      I was just referring to the backport of this. We should do this, once I backported the earlier stuff. I am already working on this (backporting smoketester, build files, initial FileChannel changes in NIO/MMapDir,...). I will open issue, once the vote succeeded and post patches and manage the backports.

      Show
      Uwe Schindler added a comment - I was just referring to the backport of this. We should do this, once I backported the earlier stuff. I am already working on this (backporting smoketester, build files, initial FileChannel changes in NIO/MMapDir,...). I will open issue, once the vote succeeded and post patches and manage the backports.
      Hide
      Furkan KAMACI added a comment -

      When I finish it I will attach the patch file.

      Show
      Furkan KAMACI added a comment - When I finish it I will attach the patch file.
      Hide
      Furkan KAMACI added a comment -

      I've finished it. Compilation and tests did not give any error. I will check it one more time and attach the patch. On the other hand I will apply changes for "lucene" module. Will anybody open a Jira issue for Solr module too or I can apply same things for Solr module too?

      Erick Erickson you are right. I've touched many many files in the code base. However I think that it will not cause any conflict (at least any real conflict) for anybody who is working on any issue. I think that the source code of Lucene became cleaner.

      Robert Muir if you want I can do same thing for "try-with resources" at another Jira issue?

      Show
      Furkan KAMACI added a comment - I've finished it. Compilation and tests did not give any error. I will check it one more time and attach the patch. On the other hand I will apply changes for "lucene" module. Will anybody open a Jira issue for Solr module too or I can apply same things for Solr module too? Erick Erickson you are right. I've touched many many files in the code base. However I think that it will not cause any conflict (at least any real conflict) for anybody who is working on any issue. I think that the source code of Lucene became cleaner. Robert Muir if you want I can do same thing for "try-with resources" at another Jira issue?
      Hide
      Erick Erickson added a comment -

      Fire away. Personally the only thing I might have that requires some work is the whole Analytics thing that I've had hanging pending getting the test failures to stop. But that's almost entirely new code so I really don't anticipate much to do.

      And don't get me wrong, I think moving to Java 7 is a fine thing. I was somehow thinking that it would be inappropriate to do that before 5.0, but clearly I was wrong. As evidence I offer the enthusiasm with which moving to Java 7 for Solr/Lucene 4.8 has been received.

      I guess what I envision at this point is that those things that have been bugging people will get attention now that the Java 6 compatibility issue is being removed. And the whole try-with thing is significant IMO, I've been tripped up by this before; Uwe rescued me.

      Thanks for putting the effort in here!

      Show
      Erick Erickson added a comment - Fire away. Personally the only thing I might have that requires some work is the whole Analytics thing that I've had hanging pending getting the test failures to stop. But that's almost entirely new code so I really don't anticipate much to do. And don't get me wrong, I think moving to Java 7 is a fine thing. I was somehow thinking that it would be inappropriate to do that before 5.0, but clearly I was wrong. As evidence I offer the enthusiasm with which moving to Java 7 for Solr/Lucene 4.8 has been received. I guess what I envision at this point is that those things that have been bugging people will get attention now that the Java 6 compatibility issue is being removed. And the whole try-with thing is significant IMO, I've been tripped up by this before; Uwe rescued me. Thanks for putting the effort in here!
      Hide
      Robert Muir added a comment -

      I've finished it. Compilation and tests did not give any error. I will check it one more time and attach the patch. On the other hand I will apply changes for "lucene" module. Will anybody open a Jira issue for Solr module too or I can apply same things for Solr module too?

      You can just supply one patch here. You can also separate it, if its easier on you. Either way.

      Robert Muir if you want I can do same thing for "try-with resources" at another Jira issue?

      Yes, we should, that one is more complicated, but there are a lot of cleanups to be done.

      Show
      Robert Muir added a comment - I've finished it. Compilation and tests did not give any error. I will check it one more time and attach the patch. On the other hand I will apply changes for "lucene" module. Will anybody open a Jira issue for Solr module too or I can apply same things for Solr module too? You can just supply one patch here. You can also separate it, if its easier on you. Either way. Robert Muir if you want I can do same thing for "try-with resources" at another Jira issue? Yes, we should, that one is more complicated, but there are a lot of cleanups to be done.
      Hide
      Furkan KAMACI added a comment -

      I'm running tests for Lucene for last time. If all tests pass I will add patch. When I finish Solr part I will start to try-with resources.

      Show
      Furkan KAMACI added a comment - I'm running tests for Lucene for last time. If all tests pass I will add patch. When I finish Solr part I will start to try-with resources.
      Hide
      Furkan KAMACI added a comment -

      Lucene part is OK. I will appy same procedure to the Solr module too.

      Show
      Furkan KAMACI added a comment - Lucene part is OK. I will appy same procedure to the Solr module too.
      Hide
      Furkan KAMACI added a comment -

      Solr module is OK. I will test it and attach whole patch.

      Show
      Furkan KAMACI added a comment - Solr module is OK. I will test it and attach whole patch.
      Hide
      Furkan KAMACI added a comment -

      I've finished removing redundant typing for trunk. 1221 files has affected from my patch. I've done it for both Lucene and Solr modules.

      I didn't use any automated tools for it and I've changed even the commented codes to avoid confusion.

      After I finished removing redundant typing I have reviewed all my changes inside 1221 files and everything seems OK. Code is compiled successfully and all tests are passed.

      My suggestion is that: if the voting ends up and result is OK to support Java 7 we should apply this patch into trunk as soon as possible. Because it includes many changes and it make Lucene/Solr code much more readable. On the other hand I don't think that it will cause conflict (at least any real conflict) for people who develops code right now.

      All in all I could have a chance to check nearly all classes of project and it was really good for me. I've noted some issues about project noted some good tips for me when I was checking all the Lucene/Solr project.

      Robert Muir you can check the code and apply the patch if vote passes.

      Show
      Furkan KAMACI added a comment - I've finished removing redundant typing for trunk. 1221 files has affected from my patch. I've done it for both Lucene and Solr modules. I didn't use any automated tools for it and I've changed even the commented codes to avoid confusion. After I finished removing redundant typing I have reviewed all my changes inside 1221 files and everything seems OK. Code is compiled successfully and all tests are passed. My suggestion is that: if the voting ends up and result is OK to support Java 7 we should apply this patch into trunk as soon as possible. Because it includes many changes and it make Lucene/Solr code much more readable. On the other hand I don't think that it will cause conflict (at least any real conflict) for people who develops code right now. All in all I could have a chance to check nearly all classes of project and it was really good for me. I've noted some issues about project noted some good tips for me when I was checking all the Lucene/Solr project. Robert Muir you can check the code and apply the patch if vote passes.
      Hide
      Robert Muir added a comment -

      Thanks Furkan, I merged the patch into trunk, i found a few missing ones (e.g. lucene/expressions, solr map-reduce contribs) but I fixed those up.

      I'll commit soon after I'm finished reviewing all the changes

      Show
      Robert Muir added a comment - Thanks Furkan, I merged the patch into trunk, i found a few missing ones (e.g. lucene/expressions, solr map-reduce contribs) but I fixed those up. I'll commit soon after I'm finished reviewing all the changes
      Hide
      Uwe Schindler added a comment -

      And now you can also backport to 4.x

      Show
      Uwe Schindler added a comment - And now you can also backport to 4.x
      Hide
      Furkan KAMACI added a comment -

      You're welcome. I know that reviewing takes a little time I also planning to apply a patch for LUCENE-3538 whenever I have time.

      Show
      Furkan KAMACI added a comment - You're welcome. I know that reviewing takes a little time I also planning to apply a patch for LUCENE-3538 whenever I have time.
      Hide
      ASF subversion and git services added a comment -

      Commit 1576755 from Robert Muir in branch 'dev/trunk'
      [ https://svn.apache.org/r1576755 ]

      LUCENE-5512: remove redundant typing (diamond operator) in trunk

      Show
      ASF subversion and git services added a comment - Commit 1576755 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1576755 ] LUCENE-5512 : remove redundant typing (diamond operator) in trunk
      Hide
      Uwe Schindler added a comment -

      Furkan KAMACI: backports should be done with svn merge and then committed. Unfortunately thats not easy to do for a non-committer. Otherwise it would be a separate patch, which is not ideal, because the merge information is lost.

      Show
      Uwe Schindler added a comment - Furkan KAMACI : backports should be done with svn merge and then committed. Unfortunately thats not easy to do for a non-committer. Otherwise it would be a separate patch, which is not ideal, because the merge information is lost.
      Hide
      Robert Muir added a comment -

      Thanks Furkan!

      Show
      Robert Muir added a comment - Thanks Furkan!
      Hide
      ASF subversion and git services added a comment -

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

      LUCENE-5512: remove redundant typing (diamond operator) in trunk

      Show
      ASF subversion and git services added a comment - Commit 1576837 from Robert Muir in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1576837 ] LUCENE-5512 : remove redundant typing (diamond operator) in trunk
      Hide
      ASF subversion and git services added a comment -

      Commit 1580271 from Steve Rowe in branch 'dev/trunk'
      [ https://svn.apache.org/r1580271 ]

      LUCENE-5512: remove redundant typing (diamond operator)

      Show
      ASF subversion and git services added a comment - Commit 1580271 from Steve Rowe in branch 'dev/trunk' [ https://svn.apache.org/r1580271 ] LUCENE-5512 : remove redundant typing (diamond operator)
      Hide
      ASF subversion and git services added a comment -

      Commit 1580272 from Steve Rowe in branch 'dev/branches/branch_4x'
      [ https://svn.apache.org/r1580272 ]

      LUCENE-5512: remove redundant typing (diamond operator)

      Show
      ASF subversion and git services added a comment - Commit 1580272 from Steve Rowe in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1580272 ] LUCENE-5512 : remove redundant typing (diamond operator)
      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:
          Unassigned
          Reporter:
          Robert Muir
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development