cloudformation-cli icon indicating copy to clipboard operation
cloudformation-cli copied to clipboard

Ensure required property is present in generated documentation

Open eduardomourar opened this issue 4 years ago • 5 comments

Issue #, if available:

Description of changes:

Whenever the required property was part of a combiner (e.g. anyOf) in a resource schema, the generated documentation would skip it. This change will ensure that required fields will be flattened and then used by the documentation generator.

You can check a documentation example here where the required property is ignored.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

eduardomourar avatar Oct 18 '20 21:10 eduardomourar

You can check a documentation example here where the required property is ignored.

Not seeing any differences after this change? None of the properties look like they're required by themselves anyways

Flattened some resource schemas in PRs linked in this issue that should have been better examples for testing, but not seeing changes from this for those previously unflattened schemas either

PatMyron avatar Dec 29 '20 19:12 PatMyron

You can find an example here that I ran with this PR's branch. The fields are required true now instead false.

eduardomourar avatar Dec 29 '20 20:12 eduardomourar

You can find an example here that I ran with this PR's branch. The fields are required true now instead false.

I'd argue only RepositoryNames is required there. The other properties are only conditionally required

PatMyron avatar Dec 29 '20 20:12 PatMyron

For reference, the official resource docs use Conditional as an explicit state when representing conditionally required (i.e. anyOf) properties:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-as-scheduledaction.html#cfn-as-scheduledaction-maxsize

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-s3-bucket-lifecycleconfig-rule.html#cfn-s3-bucket-rule-abortincompletemultipartupload

iann0036 avatar Dec 29 '20 21:12 iann0036

I agree that putting Conditional is the right way, but flattening will not be enough for that logic.

eduardomourar avatar Dec 29 '20 21:12 eduardomourar