xintongsong commented on code in PR #19888:
URL: https://github.com/apache/flink/pull/19888#discussion_r891963839
##########
flink-annotations/src/main/java/org/apache/flink/annotation/docs/Documentation.java:
##########
@@ -152,8 +152,8 @@ public String toString() {
}
/**
- * Annotation used on config option fields or REST API message headers to
exclude it from
- * documentation.
+ * Annotation used on config option fields or REST API message headers or
enum constant to
+ * exclude it from documentation.
Review Comment:
Mentioning enum constants here is probably an overkill. I'd consider it as
part of the config option use case.
##########
flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java:
##########
@@ -524,8 +524,19 @@ static String getDescription(OptionWithMetaInfo
optionWithMetaInfo) {
return null;
}
+ Set<String> enumConstantNameSet =
+ Arrays.stream(clazz.getDeclaredFields())
+ .filter(Field::isEnumConstant)
+ .filter(
+ field ->
+ !field.isAnnotationPresent(
+
Documentation.ExcludeFromDocumentation.class))
+ .map(Field::getName)
+ .collect(Collectors.toSet());
+
InlineElement[] optionDescriptions =
Arrays.stream(clazz.getEnumConstants())
+ .filter(constant ->
enumConstantNameSet.contains(((Enum) constant).name()))
Review Comment:
There's no need to store all the non-excluded constants in a set. You can
convert a `Field` into a enum constant by calling `field.get(null)`.
##########
flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java:
##########
@@ -592,6 +592,95 @@ void testConfigOptionExclusion() {
assertThat(htmlTable).isEqualTo(expectedTable);
}
+ private enum TestEnumWithExclusion {
+ VALUE_1,
+ @Documentation.ExcludeFromDocumentation
+ VALUE_2,
+ VALUE_3
+ }
+
+ private enum DescribedTestEnumWithExclusion implements DescribedEnum {
+ A(text("First letter of the alphabet")),
+ B(text("Second letter of the alphabet")),
+ @Documentation.ExcludeFromDocumentation
+ C(text("Third letter of the alphabet"));
+
+ private final InlineElement description;
+
+ DescribedTestEnumWithExclusion(InlineElement description) {
+ this.description = description;
+ }
+
+ @Override
+ public InlineElement getDescription() {
+ return description;
+ }
+ }
+
+ static class TestConfigGroupWithEnumConstantExclusion {
+
+ public static ConfigOption<TestEnumWithExclusion> enumWithExclusion =
+ ConfigOptions.key("exclude.enum")
+ .enumType(TestEnumWithExclusion.class)
+ .defaultValue(TestEnumWithExclusion.VALUE_1)
+ .withDescription("Description");
+
+ public static ConfigOption<List<TestEnumWithExclusion>>
enumListWithExclusion =
+ ConfigOptions.key("exclude.enum.list")
+ .enumType(TestEnumWithExclusion.class)
+ .asList()
+ .defaultValues(TestEnumWithExclusion.VALUE_1,
TestEnumWithExclusion.VALUE_2)
Review Comment:
Minor: It's a bit weird that `VALUE_2` is excluded but appears in the
default values.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
