Skip to content
This repository has been archived by the owner on Jan 3, 2022. It is now read-only.

add information to the schema if field is required #17

Open
tdi opened this issue Nov 9, 2016 · 3 comments
Open

add information to the schema if field is required #17

tdi opened this issue Nov 9, 2016 · 3 comments

Comments

@tdi
Copy link

tdi commented Nov 9, 2016

That would very nice if you could have information if a property is required. This would a way road for linters based on your library.

If I am not mistaken, *Expr types can be extended with a bool field.

@tdi tdi changed the title add information of fields is required to schema add information to the schema if field is required Nov 16, 2016
@tdi
Copy link
Author

tdi commented Nov 16, 2016

@crewjam CloudFormation doc seems to have either No or Yes or .. Conditional. Sometimes there is a comment, examples.

Required: Conditional. You must specify at least one of the following
                  properties: Groups, Roles, or Users.
Required: Yes
Required: No
Required: No. If you don't specify both this property and the Input, CloudWatch Events passes the entire matched event to the target.

In the scraper.go:

                if span.Text() == "Required" {
                    property.Required = span.Parent().Text()
                    property.Required = strings.TrimPrefix(property.Required, "Required: ")
                    property.Required = regexp.MustCompile("\\s+.*").ReplaceAllString(property.Required, "")
                    property.Required = strings.TrimSuffix(property.Required, ".")
                    property.Required = strings.TrimSuffix(property.Required, ":")
                }

I have no idea how it could be represented in the schema. Some idea

  1. Add a method to the ResourceProperties interface called Required, where it would have a generated map: property name -> yes/no/conditional. Then the API would be: resource.Properties.Required('Description')

@crewjam
Copy link
Owner

crewjam commented Nov 22, 2016

Sorry for the delay getting back to you. I noticed that as well when I did some exploration of this issue. I don't exactly have my head around the use case (although a linter seems like it'd be useful), so I'll defer to your judgement, but I think a tri-state enum {True, False, Conditional} or something might be appropriate. It probably isn't practical to capture or retain the text that comes after "conditional" $.02

A PR would be most welcome. :)

@tdi
Copy link
Author

tdi commented Nov 22, 2016

@crewjam Hey I can provide a PR with joy. Do you think that this kind of API would be ok for now?
source.Properties.Required('Description') -> enum

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants