You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/06/10 11:27:20 UTC
[GitHub] [lucene-solr] msokolov opened a new pull request #1563: LUCENE-9394: fix and suppress warnings
msokolov opened a new pull request #1563:
URL: https://github.com/apache/lucene-solr/pull/1563
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1563: LUCENE-9394: fix and suppress warnings
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1563:
URL: https://github.com/apache/lucene-solr/pull/1563#discussion_r438975730
##########
File path: lucene/core/src/test/org/apache/lucene/analysis/TestCharArraySet.java
##########
@@ -61,15 +61,17 @@ public void testNonZeroOffset() {
public void testObjectContains() {
CharArraySet set = new CharArraySet(10, true);
Integer val = Integer.valueOf(1);
+ @SuppressWarnings("deprecation")
+ Integer val1 = new Integer(1);
Review comment:
Add a comment that we're explicitly avoiding the Integer cache, and an `assertNotSame(val, val1)`?
##########
File path: lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/HeatmapFacetCounterTest.java
##########
@@ -33,11 +33,7 @@
import org.locationtech.spatial4j.context.SpatialContext;
import org.locationtech.spatial4j.context.SpatialContextFactory;
import org.locationtech.spatial4j.distance.DistanceUtils;
-import org.locationtech.spatial4j.shape.Circle;
-import org.locationtech.spatial4j.shape.Point;
-import org.locationtech.spatial4j.shape.Rectangle;
-import org.locationtech.spatial4j.shape.Shape;
-import org.locationtech.spatial4j.shape.SpatialRelation;
+import org.locationtech.spatial4j.shape.*;
Review comment:
wildcard import
##########
File path: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/RandomizedShapeTestCase.java
##########
@@ -183,106 +179,4 @@ private void _assertIntersect(String msg, SpatialRelation expected, Shape a, Sha
}
}
- protected void assertEqualsRatio(String msg, double expected, double actual) {
Review comment:
There appear to be more unused methods in this class, why did we keep them but not these?
##########
File path: lucene/core/src/test/org/apache/lucene/analysis/TestCharArraySet.java
##########
@@ -61,15 +61,17 @@ public void testNonZeroOffset() {
public void testObjectContains() {
CharArraySet set = new CharArraySet(10, true);
Integer val = Integer.valueOf(1);
+ @SuppressWarnings("deprecation")
+ Integer val1 = new Integer(1);
Review comment:
Add a comment that we're explicitly avoiding the Integer cache, and an `assertNotSame(val, val1)`?
##########
File path: lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/HeatmapFacetCounterTest.java
##########
@@ -33,11 +33,7 @@
import org.locationtech.spatial4j.context.SpatialContext;
import org.locationtech.spatial4j.context.SpatialContextFactory;
import org.locationtech.spatial4j.distance.DistanceUtils;
-import org.locationtech.spatial4j.shape.Circle;
-import org.locationtech.spatial4j.shape.Point;
-import org.locationtech.spatial4j.shape.Rectangle;
-import org.locationtech.spatial4j.shape.Shape;
-import org.locationtech.spatial4j.shape.SpatialRelation;
+import org.locationtech.spatial4j.shape.*;
Review comment:
wildcard import
##########
File path: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/RandomizedShapeTestCase.java
##########
@@ -183,106 +179,4 @@ private void _assertIntersect(String msg, SpatialRelation expected, Shape a, Sha
}
}
- protected void assertEqualsRatio(String msg, double expected, double actual) {
Review comment:
There appear to be more unused methods in this class, why did we keep them but not these?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov commented on pull request #1563: LUCENE-9394: fix and suppress warnings
Posted by GitBox <gi...@apache.org>.
msokolov commented on pull request #1563:
URL: https://github.com/apache/lucene-solr/pull/1563#issuecomment-642886971
Thanks for the comments, @madrob, I posted a new PR addressing them. I'm not sure how I missed all that unused code in RandomizedShapeTestCase - it's pretty bare now!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1563: LUCENE-9394: fix and suppress warnings
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1563:
URL: https://github.com/apache/lucene-solr/pull/1563#discussion_r438975730
##########
File path: lucene/core/src/test/org/apache/lucene/analysis/TestCharArraySet.java
##########
@@ -61,15 +61,17 @@ public void testNonZeroOffset() {
public void testObjectContains() {
CharArraySet set = new CharArraySet(10, true);
Integer val = Integer.valueOf(1);
+ @SuppressWarnings("deprecation")
+ Integer val1 = new Integer(1);
Review comment:
Add a comment that we're explicitly avoiding the Integer cache, and an `assertNotSame(val, val1)`?
##########
File path: lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/HeatmapFacetCounterTest.java
##########
@@ -33,11 +33,7 @@
import org.locationtech.spatial4j.context.SpatialContext;
import org.locationtech.spatial4j.context.SpatialContextFactory;
import org.locationtech.spatial4j.distance.DistanceUtils;
-import org.locationtech.spatial4j.shape.Circle;
-import org.locationtech.spatial4j.shape.Point;
-import org.locationtech.spatial4j.shape.Rectangle;
-import org.locationtech.spatial4j.shape.Shape;
-import org.locationtech.spatial4j.shape.SpatialRelation;
+import org.locationtech.spatial4j.shape.*;
Review comment:
wildcard import
##########
File path: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/RandomizedShapeTestCase.java
##########
@@ -183,106 +179,4 @@ private void _assertIntersect(String msg, SpatialRelation expected, Shape a, Sha
}
}
- protected void assertEqualsRatio(String msg, double expected, double actual) {
Review comment:
There appear to be more unused methods in this class, why did we keep them but not these?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ErickErickson commented on pull request #1563: LUCENE-9394: fix and suppress warnings
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on pull request #1563:
URL: https://github.com/apache/lucene-solr/pull/1563#issuecomment-642915566
speaking from experience, when dealing with this many changes in a big wodge, it’s _very easy_ to have some things slip through ;)
> On Jun 11, 2020, at 3:34 PM, Michael Sokolov <no...@github.com> wrote:
>
>
> Thanks for the comments, @madrob, I posted a new PR addressing them. I'm not sure how I missed all that unused code in RandomizedShapeTestCase - it's pretty bare now!
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or unsubscribe.
>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov merged pull request #1563: LUCENE-9394: fix and suppress warnings
Posted by GitBox <gi...@apache.org>.
msokolov merged pull request #1563:
URL: https://github.com/apache/lucene-solr/pull/1563
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org