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

Add "split" template function for use with strings #70

Merged
merged 5 commits into from Apr 26, 2022

Conversation

eamonnotoole
Copy link
Contributor

@eamonnotoole eamonnotoole commented Jun 10, 2021

This is an excellent piece of tooling that we are finding very helpful
as we develop a new HPE GreenLake provider (called hpegl) that we will
publish to the Provider Registry. We're using this tool to generate
documentation. We'd like the ability to set the "subcategory" field in
the resource and data-source templates. Our provider will support a
number of different services and the naming will be the following:

hpegl_< service mnemonic >_< service resource or data-source >

We want to extract the service mnemonic from the name and add that as a
"subcategory". I've added a "split" template function which is set to
strings.Split. With this template function I can add the following to a
template:

subcategory: {{ $arr := split .Name "_" }}"{{ index $arr 1 }}"

To extract the service mnemonic from the name. This would be very
helpful to us as we generate documentation.

In addition I've added .idea and vendor to .gitignore.

@hashicorp-cla
Copy link

hashicorp-cla commented Jun 10, 2021

CLA assistant check
All committers have signed the CLA.

@eamonnotoole eamonnotoole requested a review from a team as a code owner Apr 19, 2022
Copy link
Contributor

@detro detro left a comment

Hello @eamonnotoole and thanks for your contribution.

So, adding a new function for our templates is not a bad idea (there are a few more I'd like to see myself).

But there are a few more things to address in this PR before we can merge it.

  1. update the README where we document the functions
  2. update the CHANGELOG to mention the new feature that you are making available

Thank you 👍

.gitignore Outdated
@@ -16,6 +16,8 @@ tfproviderdocsgen
# OS generated files
.DS_Store

vendor
Copy link
Contributor

@detro detro Apr 21, 2022

Choose a reason for hiding this comment

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

Why did you feel the need to add this here, given we build with Go 1.17?

Copy link
Contributor Author

@eamonnotoole eamonnotoole Apr 22, 2022

Choose a reason for hiding this comment

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

I still use go mod vendor, which is probably bad practice. I can remove.

@eamonnotoole
Copy link
Contributor Author

eamonnotoole commented Apr 22, 2022

Thanks for the review @detro. I've made the changes that you requested, see the latest commit.

@eamonnotoole eamonnotoole requested a review from detro Apr 22, 2022
Copy link
Contributor

@detro detro left a comment

Sorry, but the changelog format required is different.

You need to create a brand new section for 0.8.0. See link I shared on the specific line.

CHANGELOG.md Outdated
@@ -4,6 +4,7 @@ ENHANCEMENTS:

* cmd/tfplugindocs: Use existing Terraform CLI binary if available on PATH, otherwise download latest Terraform CLI binary (https://github.com/hashicorp/terraform-plugin-docs/pull/124)
* cmd/tfplugindocs: Added `tf-version` flag for specifying Terraform CLI binary version to download, superseding the PATH lookup (https://github.com/hashicorp/terraform-plugin-docs/pull/124)
* template functions: Added the `split` command to split a string into substrings
Copy link
Contributor

@detro detro Apr 25, 2022

Choose a reason for hiding this comment

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

Sorry, I think I should have been more explicit.

The changelog format is defined here https://www.terraform.io/plugin/sdkv2/best-practices/versioning#changelog-specification .

Your change will need a new release, probably will be a 0.8.0, as it's a net new feature, but it's additive so it doesn't warrant a major release.

Copy link
Contributor Author

@eamonnotoole eamonnotoole Apr 25, 2022

Choose a reason for hiding this comment

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

Apologies @detro, see the next commit

detro
detro previously approved these changes Apr 26, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
@detro detro added this to the v0.8.0 milestone Apr 26, 2022
detro
detro previously approved these changes Apr 26, 2022
@detro
Copy link
Contributor

detro commented Apr 26, 2022

@eamonnotoole I'm not sure why, but this branch needs rebasing. Can you do that so we can land this?

Again, thank you!

@detro detro self-assigned this Apr 26, 2022
@eamonnotoole eamonnotoole requested a review from detro Apr 26, 2022
@eamonnotoole
Copy link
Contributor Author

eamonnotoole commented Apr 26, 2022

@detro I've rebased, so I think that it should be okay to merge.

@detro
Copy link
Contributor

detro commented Apr 26, 2022

Hey @eamonnotoole sorry, but GitHub is still complaining saying there are conflicts.

Can you please do an actual rebase instead of a merge? Not sure what's going on, but GitHub ain't happy.

eamonnotoole and others added 5 commits Apr 26, 2022
This is an excellent piece of tooling that we are finding very helpful
as we develop a new HPE GreenLake provider (called hpegl) that we will
publish to the Provider Registry.  We're using this tool to generate
documentation.  We'd like the ability to set the "subcategory" field in
the resource and data-source templates.  Our provider will support a
number of different services and the naming will be the following:

  hpegl_< service mnemonic >_< service resource or data-source >

We want to extract the service mnemonic from the name and add that as a
"subcategory".  I've added a "split" template function which is set to
strings.Split.  With this template function I can add the following to a
template:

  subcategory: {{ $arr := split .Name "_" }}"{{ index $arr 1  }}"

To extract the service mnemonic from the name.  This would be very
helpful to us as we generate documentation.

In addition I've added .idea and vendor to .gitignore.
@eamonnotoole
Copy link
Contributor Author

eamonnotoole commented Apr 26, 2022

@detro I've rebased, there was a conflict which I've fixed. I had to do a force push afterwards.

detro
detro approved these changes Apr 26, 2022
@detro detro merged commit 21fb40f into hashicorp:main Apr 26, 2022
1 check passed
@detro
Copy link
Contributor

detro commented Apr 26, 2022

Congrats :)

@eamonnotoole eamonnotoole deleted the add-split-function branch Apr 26, 2022
@eamonnotoole
Copy link
Contributor Author

eamonnotoole commented Apr 26, 2022

Thanks @detro. When do you think that v0.0.8 will be released?

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.

None yet

3 participants