zentol commented on issue #10748: [FLINK-15458][conf][docs] Add support for whitelisting ambiguous options URL: https://github.com/apache/flink/pull/10748#issuecomment-573345391 @tillrohrmann An annotation-based approach was my first instinct as well but that doesn't work with the existing checks the test makes. At it's core, the test does 3 things, which with the structure is obvious: ``` 1) assertDocumentedOptionsAreWellDefined(documentedOptions); 2) assertExistingOptionsAreWellDefined(existingOptions); 3) compareDocumentedAndExistingOptions(documentedOptions, existingOptions); ``` The check for documented options being well-defined is where I ran into issues; since annotations are only available for the actual ConfigOption there was no way to differentiate between normal and suffix options when you only look at the documentation. Including the prefix in the documentation can solve it, but I don't want them in the documentation as they add a lot of noise and the reporter-specific configuration page becomes a lot less concise. However, when you look at the core test structure, and think about it (as I just did trying to recall why I didn't go with annotations), then check 1) should be redundant. If all existing options are well-defined, and the documented and existing options match, then we can conclude that the documented options are also well-defined. As such, if we were to remove this check then we could in fact go with an annotation approach. I'll think about if a bit more.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
