[GitHub] [lucene-solr] dweiss commented on a change in pull request #1732: Clean up many small fixes

[GitBox <gi...@apache.org>]

dweiss commented on a change in pull request #1732:
URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468106643



##########
File path: lucene/core/src/java/org/apache/lucene/codecs/blocktree/BlockTreeTermsWriter.java
##########
@@ -709,7 +709,7 @@ private PendingBlock writeBlock(int prefixLength, boolean isFloor, int floorLead
 
           PendingTerm term = (PendingTerm) ent;
 
-          assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + term.termBytes + " prefix=" + prefix;
+          assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + new String(term.termBytes) + " prefix=" + prefix;

Review comment:
       This is wrong, uses default locale.

##########
File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java
##########
@@ -94,7 +94,7 @@
    * Create a new Analyzer, reusing the same set of components per-thread
    * across calls to {@link #tokenStream(String, Reader)}. 
    */
-  public Analyzer() {

Review comment:
       Can you not change those scopes in public API classes? This applies here and in other places -- protected changed to package-scope for source is not really an API-compatible change.

##########
File path: lucene/core/src/java/org/apache/lucene/index/DocumentsWriterFlushControl.java
##########
@@ -324,12 +324,12 @@ synchronized void doOnAbort(DocumentsWriterPerThread perThread) {
     }
   }
 
-  private void checkoutAndBlock(DocumentsWriterPerThread perThread) {
+  private synchronized void checkoutAndBlock(DocumentsWriterPerThread perThread) {

Review comment:
       These are serious changes... you're adding synchronization on core classes. I don't think they should be piggybacked on top of trivial ones - I'm sure @s1monw would chip in whether this synchronization here makes sense but he'll probably overlook if it's a bulk of trivial changes on top.

##########
File path: lucene/core/src/java/org/apache/lucene/index/DocValuesUpdate.java
##########
@@ -152,12 +152,12 @@ static BytesRef readFrom(DataInput in, BytesRef scratch) throws IOException {
     }
 
     NumericDocValuesUpdate(Term term, String field, Long value) {
-      this(term, field, value != null ? value.longValue() : -1, BufferedUpdates.MAX_INT, value != null);
+      this(term, field, value != null ? value : -1, BufferedUpdates.MAX_INT, value != null);
     }
 
 
-    private NumericDocValuesUpdate(Term term, String field, long value, int docIDUpTo, boolean hasValue) {

Review comment:
       previous version was correct camel case (upTo).




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