dweiss commented on a change in pull request #1732:
URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r471075532
##########
File path:
lucene/sandbox/src/java/org/apache/lucene/codecs/idversion/VersionBlockTreeTermsWriter.java
##########
@@ -610,7 +611,7 @@ private PendingBlock writeBlock(int prefixLength, boolean
isFloor, int floorLead
PendingEntry ent = pending.get(i);
if (ent.isTerm) {
PendingTerm term = (PendingTerm) ent;
- assert StringHelper.startsWith(term.termBytes, prefix):
"term.term=" + term.termBytes + " prefix=" + prefix;
+ assert StringHelper.startsWith(term.termBytes, prefix):
"term.term=" + Arrays.toString(term.termBytes) + " prefix=" + prefix;
Review comment:
term.toString() again?
##########
File path:
lucene/test-framework/src/java/org/apache/lucene/index/BaseTermVectorsFormatTestCase.java
##########
@@ -504,8 +504,8 @@ protected void assertEquals(RandomTokenStream tk, FieldType
ft, Terms terms) thr
}
try {
Review comment:
Since you're cleaning up I think it'd be better to use assertThrows()
with lambda...
##########
File path:
lucene/core/src/java/org/apache/lucene/index/DocumentsWriterDeleteQueue.java
##########
@@ -510,8 +510,8 @@ public String toString() {
}
private boolean forceApplyGlobalSlice() {
Review comment:
I didn't look at the code but this looks suspicious. The reordering here
changes happens-before relationship between these statements. Please leave the
order of assignment of tail as it was (inside the locked block).
##########
File path:
lucene/sandbox/src/java/org/apache/lucene/codecs/idversion/VersionBlockTreeTermsWriter.java
##########
@@ -582,7 +583,7 @@ private PendingBlock writeBlock(int prefixLength, boolean
isFloor, int floorLead
assert ent.isTerm: "i=" + i;
PendingTerm term = (PendingTerm) ent;
- assert StringHelper.startsWith(term.termBytes, prefix): "term.term="
+ term.termBytes + " prefix=" + prefix;
+ assert StringHelper.startsWith(term.termBytes, prefix): "term.term="
+ Arrays.toString(term.termBytes) + " prefix=" + prefix;
Review comment:
inconsistent with other calls (should use term.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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
