gsmiller commented on a change in pull request #578:
URL: https://github.com/apache/lucene/pull/578#discussion_r779785273
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -74,11 +74,6 @@ protected boolean useHashTable(FacetsCollector fc,
TaxonomyReader taxoReader) {
return sumTotalHits < maxDoc / 10;
}
- /** Increment the count for this ordinal by 1. */
- protected void increment(int ordinal) {
Review comment:
@jpountz that's fair. In this situation though, we've made
`IntTaxonomyFacets` publicly accessible, so I wouldn't be shocked if there are
sub-classes out there. It looks more-or-less built to allow user extensions in
its current state. I'm not opposed to making `IntTaxonomyFacets` more of an
internal implementation-detail that's not intended to support sub-classing by
our users, but if we were going to do that, I think I'd prefer we reduce
visibility of the entire class to be pkg-private.
What if we open a separate issue to reduce the visibility of this class to
pkg-private so the change gets called out in its own CHANGES entry. We could
leave the class public but marked deprecated in 9.x and reduce the vis in 10.
Maybe this is overkill though? This back-compat stuff is complex so I'm happy
to be convinced we don't need to worry about it, but I'd also like to avoid
breaking stuff for our users unnecessarily.
--
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
