Vs refactor javadoc and comment cleanup #53427
Conversation
|
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
…doc-and-comment-cleanup
…doc-and-comment-cleanup
…doc-and-comment-cleanup Conflicts: server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java
| @@ -18,5 +18,25 @@ | |||
| */ | |||
| package org.elasticsearch.search.aggregations.support; | |||
|
|
|||
| /** | |||
| * {@link AggregatorSupplier} is a place holder interface. The {@link ValuesSourceRegistry} uses this as a common interface for tools to | |||
There was a problem hiding this comment.
I'm not really sure what you mean by place holder. Like, you mean that we'll replace it with another one later? Does this interface just exist to mark things that the registry can handle?
| * subclass of {@link ValuesSource} should have a corresponding implementation of this interface. In general, new data types seeking | ||
| * aggregation support should create a top level {@link ValuesSource}, then implement this to return wrappers for the specific sources of | ||
| * values. | ||
| * {@link ValuesSourceType}s are the quantum unit of aggregations support. {@link org.elasticsearch.index.mapper.MappedFieldType}s that |
There was a problem hiding this comment.
Maybe something like `{@link ValuesSourceType}s represent data types on which aggregations can operate. On data nodes they are {@link resolved} to {@link ValueSource} instances that blah blah blah"
I'm not sure it is useful to explain too much about subclasses of ValuesSource here. I think I'd prefer to read that on ValuesSource.
| * will yield the same ValuesSource subclass. This typically happens when the underlying representation is shared, but logically the data | ||
| * are different, such as with numbers and dates. | ||
| * | ||
| * ValuesSourceTypes should be stateless, and thus immutable. We recommend that plugins define an enum for their ValuesSourceTypes, even |
There was a problem hiding this comment.
👍 on calling out stateless. Though I don't think you need to call out that that implies they are immutable.
| */ | ||
|
|
||
| /** | ||
| * The Aggregations Support package holds shared code for the aggregations framework, especially around dealing with values. |
There was a problem hiding this comment.
s/The Aggregations Support package// ? The topic of the sentence is sort of implied by its position as package javadoc.
| /** | ||
| * The Aggregations Support package holds shared code for the aggregations framework, especially around dealing with values. | ||
| * | ||
| * Key Classes |
There was a problem hiding this comment.
You probably should add html to this, just so it renders in a sane way when I mouseover the package and the like.
| * | ||
| * Key Classes | ||
| * | ||
| * {@link org.elasticsearch.search.aggregations.support.ValuesSource} and its subclasses |
There was a problem hiding this comment.
Since this is in the package do you need to fully qualify it? In general I'm fine importing things only for Javadoc if it makes the javadoc easier to read.
There was a problem hiding this comment.
AFAICT, package-info.java always requires fully qualified names in links, no idea why.
| * dependent on a user specified parameter (precision in that case). | ||
| * | ||
| * {@link org.elasticsearch.search.aggregations.support.ValuesSourceRegistry} | ||
| * ValuesSourceRegistry stores the mappings for what types are supported by what aggregations. It is configured once during startup, when |
There was a problem hiding this comment.
s/once during startup/at startup/ ?
…doc-and-comment-cleanup Conflicts: server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java
…doc-and-comment-cleanup
* ValuesSource refactor wire up missing agg (#53511) Part of refactoring ValuesSource in #42949 * ValuesSource refactoring: Wire up ExtendedStats aggregation (#53227) * Javadoc for ValuesSource related work (#53427) Co-authored-by: Andy Bristol <andy.bristol@elastic.co> Co-authored-by: Christos Soulios <1561376+csoulios@users.noreply.github.com>
This PR adds Javadoc for the
aggregations.supportpackage, as well as for several key classes in the refactor. Additionally, I cleaned up some TODO comments that either got fixed or no longer apply, renamed a few variables, oh, and deprecatedValueType.Relates to #42949