gsmiller commented on a change in pull request #149:
URL: https://github.com/apache/lucene/pull/149#discussion_r642091782



##########
File path: 
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyFacets.java
##########
@@ -16,15 +16,17 @@
  */
 package org.apache.lucene.facet.taxonomy;
 
+import static org.apache.lucene.facet.FacetsConfig.*;

Review comment:
       From what I've seen, Lucene avoid static imports (probably another 
instance of automated checks being useful).

##########
File path: 
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyFacets.java
##########
@@ -111,14 +113,25 @@ public boolean siblingsLoaded() {
   }
 
   /**
-   * Throws {@code IllegalArgumentException} if the dimension is not 
recognized. Otherwise, returns
-   * the {@link DimConfig} for this dimension.
+   * Verifies and returns {@link DimConfig} the given dimension name.

Review comment:
       I think you meant to say "returns {@link DimConfig} for the given 
dimension name." (left out the word "for")? 

##########
File path: lucene/facet/src/java/org/apache/lucene/facet/FacetsConfig.java
##########
@@ -157,6 +157,11 @@ public DimConfig getDimConfig(String dimName) {
     return dimConfig;
   }
 
+  /** Returns false if the dimension was never configured. */

Review comment:
       Maybe add a small note in the javadoc that this doesn't necessarily mean 
the dimension is "invalid", but just that it will use all "default" 
configuration? I could see a reader of this javadoc interpreting it to mean 
that the dimension is "invalid".




-- 
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:
us...@infra.apache.org



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

Reply via email to