New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SOLR-16540 Fix errors importing main and branch_9x into eclipse #1357
Conversation
8cac09e
to
14c8927
Compare
14c8927
to
6883a4b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few preliminary questions/comments.
Also, I assume these are all fixes you needed after running the ./gradlew eclipse
command?
solr/CHANGES.txt
Outdated
@@ -34,6 +34,8 @@ Other Changes | |||
Previously, the modules would come transitively. | |||
(David Smiley) | |||
|
|||
* SOLR-16540: Fix errors importing main and branch_9x into eclipse (Alex Deparvu) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we usually reserve CHANGES.txt for things a user might care about on upgrading. Internal refactors, doc changes, or dev-experience improvements don't usually get mentioned I don't think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put it under 'Other Changes', but sure I can remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
@@ -1,19 +0,0 @@ | |||
/* | |||
* Licensed to the Apache Software Foundation (ASF) under one or more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Q] What do these package-info files have to do with importing into Eclipse? Do they cause problems in some way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they have a name clash with existing files under the same package in 'source' folders. all of them are under 'test' folders, I think the current situation could be a mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding to this:
for the file solr/core/src/test/org/apache/solr/handler/admin/api/package-info.java
these are the 2 clashing
yes. and during the |
@gerlowskija gentle ping. any open questions I need to address? |
Nope, thanks for clarifying! I'm going to test it out locally today and plan to merge pending any surprises. |
thank you @gerlowskija! |
@gerlowskija do you want to mark SOLR-16540 as resolved too? |
https://issues.apache.org/jira/browse/SOLR-16540
Description
Updates needed for a clean Eclipse project import.
Updated a few git/build settings to ignore eclipse generated files.
Removed 2 unnecessary package-info files.
Renamed one test that was clashing.
Solution
Please provide a short description of the approach taken to implement your solution.
Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
Checklist
Please review the following and check all that apply:
main
branch../gradlew check
.