[GitHub] [lucene] dweiss commented on a change in pull request #360: LUCENE-10155: Refactor TestMultiMMap into a BaseChunkedDirectoryTestCase

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

dweiss commented on a change in pull request #360:
URL: https://github.com/apache/lucene/pull/360#discussion_r723900036



##########
File path: lucene/core/src/java/org/apache/lucene/store/ByteBuffersDataOutput.java
##########
@@ -137,6 +137,14 @@ public ByteBuffersDataOutput(
     this.blockReuse = Objects.requireNonNull(blockReuse, "Block reuse must not be null.");
   }
 

Review comment:
       Just a note. I wonder why the restrictions in the public constructor are there - I can't remember the motivation for these. I'm guessing they're there because small-ish blocks don't make much sense for real-life applications in production and this guards against such insane input arguments. On the other hand, why not leave the decision to whoever is using the class (such as tests in this case. Can't make my mind whether the package-private constructor makes more sense than just opening up the range of arguments to a full spectrum of allowed values. You decide. :)




-- 
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@lucene.apache.org

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