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.

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

Added missing import.sh files and generate docs #244

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

fantapop
Copy link
Collaborator

The docs plugin adds a section to each each resource indicating the support of Import based on the existence of an import.sh example. This file was missing for some resources despite the actual support of the Import action. The missing files are added here and docs are regenerated.

Commit checklist

  • Changelog
  • Doc gen (make generate)
  • Integration test(s)
  • Acceptance test(s)
  • Example(s)

@fantapop fantapop force-pushed the fantapop/CC-30360-add-missing-import-docs branch 2 times, most recently from 61e17cc to 7d4eb22 Compare October 29, 2024 23:32
The docs plugin adds a section to each each resource indicating the
support of Import based on the existence of an import.sh example. This
file was missing for some resources despite the actual support of the
Import action. The missing files are added here and docs are
regenerated.
@fantapop fantapop force-pushed the fantapop/CC-30360-add-missing-import-docs branch from 7d4eb22 to 34ba219 Compare October 29, 2024 23:34
@fantapop
Copy link
Collaborator Author

@kathancox Most of this wording is built into the framework. I could change the names however and there are a few comments in this PR that serve as documentation. It would be much appreciated if you could take a peek!

Copy link

@kathancox kathancox left a comment

Choose a reason for hiding this comment

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

This lgtm, the only question I had (mainly because I'm curious): the .example part of the command what does a user put there? I saw that the issuer command had .my_issuer in comparison to the other command examples, so that stood out!

@fantapop
Copy link
Collaborator Author

This lgtm, the only question I had (mainly because I'm curious): the .example part of the command what does a user put there? I saw that the issuer command had .my_issuer in comparison to the other command examples, so that stood out!

@kathancox this basically like the path of a resource. Usually resource type + resource name. In the example here it would be aws_instance.web:
https://developer.hashicorp.com/terraform/language/resources/syntax#resource-syntax

The naming of the resource in these isn't something we have a strong pattern for at the moment. I've noticed a lot of providers use the name "example" as the second part. Obviously for real configuration, that is not a name that should be used. Mostly what I've done is to try and pick realistic names but given that people use different naming schemes for their resources, it might not be what people use. Also sometimes it's hard to come up with good names. We could also consider trying to match the name in the config examples with the name in the import example. You can see in this one I used:
cockroach_allow_list.vpn for the config example and
cockroach_allow_list.home_office for the import example

I'm not opposed to using "example" for all of them if you think that will help from a documentation perspective.

@kathancox
Copy link

This lgtm, the only question I had (mainly because I'm curious): the .example part of the command what does a user put there? I saw that the issuer command had .my_issuer in comparison to the other command examples, so that stood out!

@kathancox this basically like the path of a resource. Usually resource type + resource name. In the example here it would be aws_instance.web: https://developer.hashicorp.com/terraform/language/resources/syntax#resource-syntax

The naming of the resource in these isn't something we have a strong pattern for at the moment. I've noticed a lot of providers use the name "example" as the second part. Obviously for real configuration, that is not a name that should be used. Mostly what I've done is to try and pick realistic names but given that people use different naming schemes for their resources, it might not be what people use. Also sometimes it's hard to come up with good names. We could also consider trying to match the name in the config examples with the name in the import example. You can see in this one I used: cockroach_allow_list.vpn for the config example and cockroach_allow_list.home_office for the import example

I'm not opposed to using "example" for all of them if you think that will help from a documentation perspective.

@fantapop Got it. So, I actually prefer the more descriptive, placeholder resource name e.g., .my_issuer or .home_office compared to .example. I don't think this is a big deal to change though. But, if you're so inclined and can think of more helpful placeholders than "example" it could be worth considering.

Thanks for the explanation there.

@fantapop
Copy link
Collaborator Author

Thanks for your thoughts @kathancox. In that case, I think we should consider doing a sweep for this as part of a later PR. I'd rather just get this one merged for now.

@fantapop fantapop merged commit b78f543 into main Oct 30, 2024
4 checks passed
@fantapop fantapop deleted the fantapop/CC-30360-add-missing-import-docs branch October 30, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants