vigyasharma commented on a change in pull request #633:
URL: https://github.com/apache/lucene/pull/633#discussion_r822195662
##########
File path: lucene/core/src/java/org/apache/lucene/index/NoMergePolicy.java
##########
@@ -39,6 +40,11 @@ public MergeSpecification findMerges(
return null;
}
+ @Override
+ public MergeSpecification findMerges(List<CodecReader> readers) throws
IOException {
+ return null;
Review comment:
In my current impl, addIndexes simply completes without adding anything
if the API returns null. I now realize, this is misleading for the user.
Callers of addIndexes may not know about the configured merge policy. I should
probably throw an exception if I get a null here?
On similar lines, what should we do if the API call had some CodecReaders,
but findMerges returns an empty spec.? Should we error out? (Will add a log for
this either way.)
Re: letting the API return null, I see that other variants of `findMerges`
in this policy, all return `null`, and all callers of findMerges seem to be
doing the null checks. Personally, I prefer disallowing the null return values,
but I wanted to keep this in sync with the other variants.
--
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-unsubscr...@lucene.apache.org
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
