Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Place default ID attribute in Read Only group #46

Closed
wants to merge 7 commits into from
Closed

Place default ID attribute in Read Only group #46

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6cc7a3
Place default ID attribute in Read Only group
nmuesch Mar 4, 2021
40b0963
Remove extra check of empty ID description
nmuesch Mar 4, 2021
96fb961
Merge branch 'main' into nick/default_id
nmuesch Apr 26, 2022
5e66e53
Address feedback
nmuesch Apr 26, 2022
af5eaa6
Update CHANGELOG.md
detro May 3, 2022
3ad1834
Update schemamd/render.go
detro May 3, 2022
9ee2af1
Update render.go
May 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 11 additions & 8 deletions schemamd/render.go
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ func Render(schema *tfjson.Schema, w io.Writer) error {
}

type groupFilter struct {
name string
topLevelTitle string
nestedTitle string

Expand All @@ -34,9 +35,9 @@ type groupFilter struct {

var (
groupFilters = []groupFilter{
{"### Required", "Required:", childIsRequired},
{"### Optional", "Optional:", childIsOptional},
{"### Read-Only", "Read-Only:", childIsReadOnly},
{"Required", "### Required", "Required:", childIsRequired},
{"Optional", "### Optional", "Optional:", childIsOptional},
{"Read Only", "### Read-Only", "Read-Only:", childIsReadOnly},
}
)

Expand All @@ -57,10 +58,6 @@ func writeAttribute(w io.Writer, path []string, att *tfjson.SchemaAttribute, gro
return nil, err
}

if name == "id" && att.Description == "" {
att.Description = "The ID of this resource."
}

err = WriteAttributeDescription(w, att, false)
if err != nil {
return nil, err
Expand Down Expand Up @@ -161,7 +158,13 @@ func writeBlockChildren(w io.Writer, parents []string, block *tfjson.SchemaBlock
childBlock := block.NestedBlocks[n]
childAtt := block.Attributes[n]
for i, gf := range groupFilters {
if gf.filter(childBlock, childAtt) {
if n == "id" && childAtt.Description == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd do a lowercase comparison with id, just to stay on the safe side

if gf.name == "Read Only" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a new field name for group filter, just to support this comparison, wouldn't it be easier to just do a string contains Read-Only?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the current implementation to check this. I think my original rationale was trying to give it an enum style ID that could be referenced (in case it needed to be for any other purposes) But I'm totally fine just checking against an existing field.

childAtt.Description = "The ID of this resource."
groups[i] = append(groups[i], n)
goto NextName
}
} else if gf.filter(childBlock, childAtt) {
groups[i] = append(groups[i], n)
goto NextName
}
Expand Down