You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/09/22 19:33:53 UTC
[GitHub] [solr] risdenk opened a new pull request, #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
risdenk opened a new pull request, #1039:
URL: https://github.com/apache/solr/pull/1039
https://issues.apache.org/jira/browse/SOLR-16427
Builds on https://github.com/apache/solr/pull/1037
Enables the following errorprone rules:
*
Most of the fixes are straight forward - use .equals and Objects.equals where appropriate. Used contentEquals for char sequences. Make sure to compare Instant instead of just Date. Ensure proper string format number of arguments. Make sure to use proper Array methods for hashCode and toString
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1256108074
THis is failing...
```
gradlew :solr:core:test --tests "org.apache.solr.core.TestCoreContainer.classMethod" -Ptests.jvms=4 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=7A555DEB6D0F059A -Ptests.file.encoding=UTF-8
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a diff in pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1039:
URL: https://github.com/apache/solr/pull/1039#discussion_r978178058
##########
solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java:
##########
@@ -71,15 +71,16 @@ public RepositoryManager(HttpSolrClient solrClient, PackageManager packageManage
this.solrClient = solrClient;
}
- public List<SolrPackage> getPackages() {
- List<SolrPackage> list = new ArrayList<>(getPackagesMap().values());
+ public List<org.apache.solr.packagemanager.SolrPackage> getPackages() {
Review Comment:
oh wow... I see the problem with SolrPackage... Is it a Bean? Is it a class? Too bad we don't have `o.a.s.client.solrj.request.beans.SolrPackageBean` as a existing naming pattern... Was taht a thing at one time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk merged pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk merged PR #1039:
URL: https://github.com/apache/solr/pull/1039
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1256195026
> `./gradlew :solr:core:test --tests "org.apache.solr.core.TestCoreContainer"`
@epugh I'm not seeing that test fail. I ran it ~10 times and no failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1287277826
@dsmiley / @epugh I force pushed this branch to reapply these changes on top of https://github.com/apache/solr/pull/953 and break up each commit into one error-prone rule. Hopefully that makes it easier to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1289310500
I didn't make any changes to the underlying code - just updated all the commit messages. I don't plan on squashing these so its easier to revert any individual commits if necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a diff in pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1039:
URL: https://github.com/apache/solr/pull/1039#discussion_r978126229
##########
solr/benchmark/src/java/org/apache/solr/bench/generators/Lists.java:
##########
@@ -32,17 +31,13 @@ public final class Lists {
private Lists() {}
static <T> Gen<List<T>> listsOf(Gen<T> generator, Gen<Integer> sizes) {
- return listsOf(generator, arrayList(), sizes).mix(listsOf(generator, linkedList(), sizes));
Review Comment:
was going to comment on this, because of the removal of `.mix`, but I think what you are saying is that the `.mix` doesn't actually do anything/matter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1256674360
Thanks for this.
It'd help reviewing if you did the work in separate commits. That way I could look at the commits and maybe review some commits of interest to me.
For example inlining deprecated methods on SolrException could have been its own commit. This in particular feels different in nature from the other changes that it should be in some other PR, actually.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1275052269
> It'd help reviewing if you did the work in separate commits. That way I could look at the commits and maybe review some commits of interest to me.
> For example inlining deprecated methods on SolrException could have been its own commit. This in particular feels different in nature from the other changes that it should be in some other PR, actually.
yea will admit got a bit overzealous fixing things in some of these files. I'll see if I can clean it up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1286057920
yea I figured it would conflict after https://github.com/apache/solr/pull/953. gives me a chance to separate out the commits as @dsmiley rightly pointed out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1039:
URL: https://github.com/apache/solr/pull/1039#discussion_r978171652
##########
gradle/validation/error-prone.gradle:
##########
@@ -74,17 +74,14 @@ allprojects { prj ->
'-Xep:StaticAssignmentInConstructor:OFF', // we assign SolrTestCaseJ4.configString in many tests, difficult to untangle
'-Xep:ComparableType:OFF', // SolrTestCaseJ4.Doc and Fld are messy
- '-Xep:AlmostJavadoc:OFF',
Review Comment:
https://errorprone.info/bugpatterns
there is a very big list and grows/changes each time errorprone is updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1289855920
part 3 continues here: https://github.com/apache/solr/pull/1116
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1282941670
I'm going to come back to this after https://github.com/apache/solr/pull/953 is merged. I'm expecting some conflicts based on changes I shouldn't have made in this PR that are made in https://github.com/apache/solr/pull/953
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1255680211
I've looked at about half the changes... It all *seems* good... I'm going to try and run the tests before I head to bed on this branch...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1256103905
Your most recent fix JUST fixed the test error I was getting...
`./gradlew :solr:core:test --tests "org.apache.solr.handler.component.DistributedFacetPivotLargeTest"` passes now for me!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1287278907
I'm not 100% sure I got all the changes from my previous branch - I'm running all the tests now and will potentially force push again to fix any minor things I missed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a diff in pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1039:
URL: https://github.com/apache/solr/pull/1039#discussion_r978126733
##########
gradle/validation/error-prone.gradle:
##########
@@ -74,17 +74,14 @@ allprojects { prj ->
'-Xep:StaticAssignmentInConstructor:OFF', // we assign SolrTestCaseJ4.configString in many tests, difficult to untangle
'-Xep:ComparableType:OFF', // SolrTestCaseJ4.Doc and Fld are messy
- '-Xep:AlmostJavadoc:OFF',
Review Comment:
I like that in Rubocop, each rule is added to the list and is either set to on or off, so you can see everything you are enforcing, versus having things that aren't listed as being ON. This is totally a stylistic thing...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1039:
URL: https://github.com/apache/solr/pull/1039#discussion_r978615551
##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/InnerJoinStream.java:
##########
@@ -32,10 +32,14 @@
* @since 6.0.0
*/
public class InnerJoinStream extends BiJoinStream implements Expressible {
+ @SuppressWarnings("JdkObsolete")
+ private final LinkedList<Tuple> joinedTuples = new LinkedList<>();
- private LinkedList<Tuple> joinedTuples = new LinkedList<>();
- private LinkedList<Tuple> leftTupleGroup = new LinkedList<>();
- private LinkedList<Tuple> rightTupleGroup = new LinkedList<>();
+ @SuppressWarnings("JdkObsolete")
+ private final LinkedList<Tuple> leftTupleGroup = new LinkedList<>();
+
+ @SuppressWarnings("JdkObsolete")
Review Comment:
Note to self @risdenk: Create follow up jira to try to remove these.
##########
solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java:
##########
@@ -71,15 +71,16 @@ public RepositoryManager(HttpSolrClient solrClient, PackageManager packageManage
this.solrClient = solrClient;
}
- public List<SolrPackage> getPackages() {
- List<SolrPackage> list = new ArrayList<>(getPackagesMap().values());
+ public List<org.apache.solr.packagemanager.SolrPackage> getPackages() {
Review Comment:
Actually looks like the pattern is `...beans.*Payload` so going to change the `SolrPackage` -> `PackagePayload`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1287303290
```
solr git:(SOLR-16427-part-2) ./gradlew check -Pvalidation.errorprone=true
...
BUILD SUCCESSFUL in 21m 31s
586 actionable tasks: 215 executed, 371 up-to-date
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #1039:
URL: https://github.com/apache/solr/pull/1039#issuecomment-1289457488
+1 thanks Kevin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1039: SOLR-16427: Evaluate and fix errorprone rules - part 2
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1039:
URL: https://github.com/apache/solr/pull/1039#discussion_r978661778
##########
solr/benchmark/src/java/org/apache/solr/bench/generators/Lists.java:
##########
@@ -32,17 +31,13 @@ public final class Lists {
private Lists() {}
static <T> Gen<List<T>> listsOf(Gen<T> generator, Gen<Integer> sizes) {
- return listsOf(generator, arrayList(), sizes).mix(listsOf(generator, linkedList(), sizes));
Review Comment:
I'm going to revert these benchmark changes since doesn't seem to be necessary. https://github.com/quicktheories/QuickTheories/blob/master/core/src/main/java/org/quicktheories/core/Gen.java#L168 btw says mix is just combining the links.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org