Michael McCandless commented on LUCENE-8248:

Hmm maybe instead we should remove the {{final}} from 
{{getMaxCFSSegmentSizeMB}} and override both in the wrapper?

Otherwise sneaky things could go wrong e.g. if you subclass 
{{MergePolicyWrapper}} and delegate to another merge policy that makes 
decisions based on the {{maxCFSSizeMB.}}  The caller would call 
{{setMaxCFSSizeMB}} but the wrapped merge policy wouldn't see the change I 

Also, I do think we should do the rename here to make it consistent.  We can do 
a hard rename for 8.x (master) and then on backport to 7.x we can leave a 
deprecated minimal {{MergePolicyWrapper}} just subclassing the new 

> Make MergePolicy.setMaxCFSSegmentSizeMB final
> ---------------------------------------------
>                 Key: LUCENE-8248
>                 URL: https://issues.apache.org/jira/browse/LUCENE-8248
>             Project: Lucene - Core
>          Issue Type: Wish
>          Components: core/index
>            Reporter: Mike Sokolov
>            Priority: Minor
>         Attachments: MergePolicy.patch
> MergePolicy.getMaxCFSSegmentSizeMB is final, but the corresponding setter is 
> not, which means that overriding it with anything other than a trivial 
> delegation can only lead to confusion.
> The patch makes the method final and removes the trivial implementations from 
> MergePolicyWrapper and NoMergePolicy.
> [~mikemccand] also pointed out that the class name is nonstandard for similar 
> adapter classes in Lucene, which are usually Filter*.java. Personally I was 
> looking for MergePolicyAdapter, but if there is a prevailing convention here 
> around Filter, does it make sense to change this class's name to 
> FilterMergePolicy?

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to