[GitHub] [lucene] apanimesh061 edited a comment on pull request #412: LUCENE-10197: UnifiedHighlighter should use builders for thread-safety

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

apanimesh061 edited a comment on pull request #412:
URL: https://github.com/apache/lucene/pull/412#issuecomment-953333323


   > It looks good at a glance... you/I will see better if you update one of the clients that might want to subclass with extra configuration. Is there any or is this builder subclassing issue entirely hypothetical at this point? I suspect only hypothetical. We'll want nice Javadocs on the builder setters since this is where consumers/clients will see it. We can merely move the docs there from the existing locations, and add javadoc references pointing to the builder from the existing fields/enum values as desired.
   
   @dsmiley Great. I added a unit test (just for demo) and a class `SubUnifiedHighlighter` in `TestUnifiedHighlighter.java` where I've added a new test field and also tested it. It does look right to me since it is able to use the new field and also fields from parent class.
   
   I can add some unit tests to test the new builder, then I can focus on modifying the javadocs in order to introduce the builder.
   


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