Allowing single word resources to use templates#147
Conversation
bflad
left a comment
There was a problem hiding this comment.
Sad that we need this, but looks good to me 🚀
internal/provider/util.go
Outdated
| // are identical after file extensions have been stripped from the | ||
| // latter. This allows single word resources (e.g., http) to use | ||
| // templates (e.g., http.md.tmpl). | ||
| func resourceName(shortName, relFile string) string { |
There was a problem hiding this comment.
Could we add a quick unit test for this?
internal/provider/generate.go
Outdated
| switch relDir { | ||
| case "data-sources/": | ||
| resName := shortName + "_" + removeAllExt(relFile) | ||
| resName := resourceName(shortName, relFile) |
There was a problem hiding this comment.
Question: isn't shortName the Provider shortname here?
In the case of the HTTP provider (through which you identified the issue), I believe the http refers to the resource. Otherwise if, for example, you decide to create a second resource in this provider (say, for the sake of it, https), you won't be able to match it OR it would match the wrong file.
I'll leave a comment in the unit test below.
internal/provider/util_test.go
Outdated
| templateFileName string | ||
| expectedResourceName string | ||
| }{ | ||
| { |
There was a problem hiding this comment.
Can you add another case where you have a resource named (just for kicks) https, using the same format used by the http provider?
I say this because the http provider is here not respecting the convention: <provider>_<resource> when exposed.
There was a problem hiding this comment.
@detro you mean along the lines of:
{
"provider short name different from template file name",
"https",
"http.md.tmpl",
"https_http",
},
There was a problem hiding this comment.
{
"provider short name different from template file name",
"http",
"https.md.tmpl",
"https",
},
What I'm expecting is that this test would fail, because the provider name is http, but the resource name is https.
There was a problem hiding this comment.
@detro I've refactored following discussion.
If the |
|
I'd be a BIG fan of |
internal/provider/util_test.go
Outdated
| { | ||
| "provider short name concatenated with same template file name matches schema name", | ||
| map[string]*tfjson.Schema{ | ||
| "tls_tls": {}, | ||
| }, | ||
| "tls", | ||
| "tls.md.tmpl", | ||
| &tfjson.Schema{}, | ||
| "tls_tls", | ||
| }, |
There was a problem hiding this comment.
Nit: While Go allows you to omit field names, I personally find it a little more difficult to read the structs once the struct definition is "beyond the fold" since its based on ordering. e.g. my brain is like what is the 3rd field defined as "tls" here? I have to scroll up, see providerShortName, scroll back down
There was a problem hiding this comment.
Fair point. I forget that GoLand is doing this for me:
Have added field names and refactored to use map[string]struct{} for the test cases.
| NEW FEATURES: | ||
|
|
||
| * cmd/tflugindocs: Additional CLI arguments `provider-name`, `rendered-provider-name`, `rendered-website-dir`, `examples-dir`, `website-temp-dir`, and `website-source-dir`. These allow to further customise generated doc ([#95](https://github.com/hashicorp/terraform-plugin-docs/pull/95)). | ||
| * cmd/tfplugindocs: Additional CLI arguments `provider-name`, `rendered-provider-name`, `rendered-website-dir`, `examples-dir`, `website-temp-dir`, and `website-source-dir`. These allow to further customise generated doc ([#95](https://github.com/hashicorp/terraform-plugin-docs/pull/95)). |
There was a problem hiding this comment.
those are definitely typos I made 🤦
There was a problem hiding this comment.
No worries. I like that we're keeping an eye out for each other 👍
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Currently, it is not possible to have single word
resourcesordata sources(e.g.,http) use templates in the generation of docs. This PR addresses that by allowing single word resources and data sources to use a matching template (e.g.,httpdata source usinghttp.md.tmpltemplate).An error is now returned if a resource cannot be found using either the provider short name or the provider short name concatenated with the template file name.