-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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.
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
[WIP] command/jsonprovider: export providers schemas to json #20446
Conversation
I'm going to work on the bulk of the documentation in a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor feedback inline, but nothing seems blocking if you'd prefer to merge as-is and think about the feedback in possible future PRs.
command/jsonprovider/block.go
Outdated
case "NestingMap": | ||
ret.NestingMode = "map" | ||
default: | ||
// unpossible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move the statement from "nestingModeInvalid"
down here and remove that case? I think it being literally nestingModeInvalid
and it being some value that isn't in the enum at all are... both invalid, really.
command/jsonprovider/block.go
Outdated
MaxItems: uint64(nestedBlock.MaxItems), | ||
} | ||
|
||
switch nestedBlock.Nesting.String() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was surprised to see the .String()
call here vs. switching directly on the nestedBlock.Nesting
value. Could you add a comment explaining it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll just drop the .String()
and check the nested mode directly - I had doubts when I wrote that but forgot to follow up.
At this point, json output is the only option for the `terraform providers schema` command. To maintain consistency with other terraform commands, and with future enhancements in mind, this should be a requireed flag.
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
TODO: