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