[GitHub] [lucene] mikemccand commented on a change in pull request #25: LUCENE-9385: Add option to skip indexing facet drill down terms

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

mikemccand commented on a change in pull request #25:
URL: https://github.com/apache/lucene/pull/25#discussion_r597761931



##########
File path: lucene/facet/src/java/org/apache/lucene/facet/FacetsConfig.java
##########
@@ -444,12 +478,6 @@ private void processAssocFacetFields(
         FacetsConfig.DimConfig ft = getDimConfig(field.dim);
 
         // Drill down:
-        int start;

Review comment:
       Good catch!  It's crazy our static tooling can't ... help us remove such code :)

##########
File path: lucene/facet/src/java/org/apache/lucene/facet/FacetsConfig.java
##########
@@ -87,6 +89,8 @@
      * True if drilling down by a whole dimension, to match all documents that had any value for
      * this dimension, is necessary (default is true)
      */
+    // nocommit should this (control for only label / dim name indexing) be deprecated to favor the

Review comment:
       I think there three use cases we want to enable, which we are now doing with two booleans:
   
     * Index no drill down terms
     * Index dimension and full path drill down
     * Index only full path drill down
   
   I think the 4th logical option (Index only dimension) is very weird and we don't need to make that easy?
   
   So maybe we could make an `enum` for these three, instead of two booleans?




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

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