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

Terraform Provider Documentation Alignment #280

Merged
merged 30 commits into from
Oct 23, 2024
Merged

Conversation

lornest
Copy link
Contributor

@lornest lornest commented Oct 16, 2024

Hey @jdamata!

This is just a small change to align the documentation with the expected format for a Terraform Provider (see examples in the Hashicorp generator).

This has no functional impact, but is not purely selfless - a number of tools that have been built around the TF ecosystem rely on these metadata - for example, I'm trying to run Upjet to generate a Kubernetes controller based off of your provider, which seems to scrape this metadata from the docs.

Thanks!

@tbutler-qontigo
Copy link
Contributor

Nice idea...but how do we know that this is valid for said format...or indeed what stops the next PR just messing this up?
You are potentially creating a maintenance burden that nobody will respect because nobody really cares :)
Is there some sort of linting or other automated tool you could add that would run for PRs to validate that the docs are still compliant with the required format?

Otherwise it means we are taking your word for it that they are valid today and you are hoping that going forward, others will ensure any new/changed docs also comply

@lornest
Copy link
Contributor Author

lornest commented Oct 16, 2024

Sure, I can look at generating directly from this: https://github.com/hashicorp/terraform-plugin-docs and adding the relevant descriptions etc that are already in the hand cranked docs (I manually added the metadata to avoid the potential over generation).

Could then look at using the tfplugindocs validate as part of PR actions?

@tbutler-qontigo
Copy link
Contributor

That would make sense to me though @jdamata would have final say since it is his repo!
You might be able to take inspiration from https://github.com/terraform-docs/gh-actions which does for terraform modules what you want to do for terraform plugins.

@lornest
Copy link
Contributor Author

lornest commented Oct 17, 2024

Okay @tbutler-qontigo, @jdamata, I've now implemented tfplugindocs approach to generate documentation from the provider source code (resources and data sources), templates and examples.

I've taken the descriptions from the previous documentation to preserve as much as possible.

Where things were not possible to be generated from the source, I've used templates.

I've also implemented the tfplugindocs-check workflow. This checks that the documentation is up-to-date. It will fail if make docs is not run before committing, and something is added that makes the docs out of date. I've updated the README.md to add the need to run make docs before committing (NOTE: it's possible to add an 'on pull request' workflow for this, if you'd like me to take a look at this? - it requires the contributor setting a GitHub token secret to the workflow can generate the docs and commit them to their pull request if there are diffs)

Let me know your thoughts.

.DS_Store Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@jdamata
Copy link
Owner

jdamata commented Oct 17, 2024

Thanks for the contribution @lornest. I added some small nitpick comments.

All the documentation being shifted around is a lot to read through and verify its all good. Ill try to read through all the doc diffs and catch anything missed

@freeranger
Copy link
Collaborator

I've updated the README.md to add the need to run make docs before committing (NOTE: it's possible to add an 'on pull request' workflow for this, if you'd like me to take a look at this? - it requires the contributor setting a GitHub token secret to the workflow can generate the docs and commit them to their pull request if there are diffs)

Let me know your thoughts.

Great stuff @lornest but as you say, you have now introduced an additional step for the developer to have to run an manual step to generated automated documentation - it seems like it would be better to do as you suggest above - ie to have the PR do this step automatically.
That way the contributor has no extra steps to perform to get the documentation up to date.
I’m not sure what you mean about the contributor setting a token - isnt the built in token enough?

what do you think @jdamata

templates/debugging.md Outdated Show resolved Hide resolved
templates/debugging.md Outdated Show resolved Hide resolved
@jdamata
Copy link
Owner

jdamata commented Oct 18, 2024

I've updated the README.md to add the need to run make docs before committing (NOTE: it's possible to add an 'on pull request' workflow for this, if you'd like me to take a look at this? - it requires the contributor setting a GitHub token secret to the workflow can generate the docs and commit them to their pull request if there are diffs)
Let me know your thoughts.

Great stuff @lornest but as you say, you have now introduced an additional step for the developer to have to run an manual step to generated automated documentation - it seems like it would be better to do as you suggest above - ie to have the PR do this step automatically. That way the contributor has no extra steps to perform to get the documentation up to date. I’m not sure what you mean about the contributor setting a token - isnt the built in token enough?

what do you think @jdamata

yah i agree and that would be sweet. @lornest is this something you could take a stab at?

@lornest
Copy link
Contributor Author

lornest commented Oct 18, 2024

@jdamata will address all of the above comments from you and the other contributors/maintainers and add the on PR workflow today.

Thanks for the feedback everyone 👍

@lornest lornest marked this pull request as draft October 18, 2024 08:14
@lornest
Copy link
Contributor Author

lornest commented Oct 21, 2024

Hey @jdamata and co!

I've added 2 workflows and tested them on my local fork.

First, when someone creates a PR with changes to the provider code (/sonarqube/...) or any files that are used in the generation of docs (templates, examples, makefile) then the action generates updated docs and pushes them to the PR branch.

There's also another workflow that checks the documents are up-to-date on push to master. This is pretty much belt-and-braces if all code gets into master via PRs, but it might be useful if anyone from the contributors team ever pushes to master directly.

Let me know if there's anything else!

@jdamata
Copy link
Owner

jdamata commented Oct 21, 2024

Hey @jdamata and co!

I've added 2 workflows and tested them on my local fork.

First, when someone creates a PR with changes to the provider code (/sonarqube/...) or any files that are used in the generation of docs (templates, examples, makefile) then the action generates updated docs and pushes them to the PR branch.

There's also another workflow that checks the documents are up-to-date on push to master. This is pretty much belt-and-braces if all code gets into master via PRs, but it might be useful if anyone from the contributors team ever pushes to master directly.

Let me know if there's anything else!

small nit in a comment. otherwise LGTM

lornest and others added 2 commits October 22, 2024 15:10
* testing workflow after changing actions to SHAs

* [bot] docs: tfplugindocs generate

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@lornest lornest marked this pull request as ready for review October 22, 2024 14:31
@lornest
Copy link
Contributor Author

lornest commented Oct 23, 2024

Thanks all, I've completed the updates as per the feedback

@jdamata jdamata merged commit d806038 into jdamata:master Oct 23, 2024
12 checks passed
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.

4 participants