apanimesh061 commented 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. 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. -- 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
