search

apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.61k stars 1.02k forks source link

Remove redundant typing (diamond operator) in trunk [LUCENE-5512] #6575

Closed asfimport closed 10 years ago

asfimport commented 10 years ago

Migrated from LUCENE-5512 by Robert Muir (@rmuir), resolved Mar 12 2014 Attachments: LUCENE-5512.patch (versions: 3)

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

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

asfimport commented 10 years ago

Robert Muir (@rmuir) (migrated from JIRA)

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).

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

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

asfimport commented 10 years ago

Erick Erickson (@ErickErickson) (migrated from JIRA)

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...

asfimport commented 10 years ago

Robert Muir (@rmuir) (migrated from JIRA)

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

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

asfimport commented 10 years ago

Robert Muir (@rmuir) (migrated from JIRA)

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

asfimport commented 10 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

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!

asfimport commented 10 years ago

Robert Muir (@rmuir) (migrated from JIRA)

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

asfimport commented 10 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

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.

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

When I finish it I will attach the patch file.

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

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?

@ErickErickson 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.

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

asfimport commented 10 years ago

Erick Erickson (@ErickErickson) (migrated from JIRA)

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!

asfimport commented 10 years ago

Robert Muir (@rmuir) (migrated from JIRA)

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.

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

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.

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

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

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

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

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

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.

@rmuir you can check the code and apply the patch if vote passes.

asfimport commented 10 years ago

Robert Muir (@rmuir) (migrated from JIRA)

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

asfimport commented 10 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

And now you can also backport to 4.x :-)

asfimport commented 10 years ago

Furkan Kamaci (migrated from JIRA)

You're welcome. I know that reviewing takes a little time :) I also planning to apply a patch for #4612 whenever I have time.

asfimport commented 10 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1576755 from @rmuir in branch 'dev/trunk' https://svn.apache.org/r1576755

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

asfimport commented 10 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

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.

asfimport commented 10 years ago

Robert Muir (@rmuir) (migrated from JIRA)

Thanks Furkan!

asfimport commented 10 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1576837 from @rmuir in branch 'dev/branches/branch_4x' https://svn.apache.org/r1576837

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

asfimport commented 10 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1580271 from @sarowe in branch 'dev/trunk' https://svn.apache.org/r1580271

LUCENE-5512: remove redundant typing (diamond operator)

asfimport commented 10 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1580272 from @sarowe in branch 'dev/branches/branch_4x' https://svn.apache.org/r1580272

LUCENE-5512: remove redundant typing (diamond operator)

asfimport commented 10 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

Close issue after release of 4.8.0