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

feat(terraform_docs): Add terraform-docs default markers support and describe how to migrate to them #609

Merged

Conversation

jonmcewen
Copy link
Contributor

@jonmcewen jonmcewen commented Dec 20, 2023

Put an x into the box if that apply:

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

Allow users to switch to the standard insertion markers used by terraform-docs, to enable CI jobs to check that docs have been updated.

How can we test changes

Based on this suggestion for testing hooks: #25 (comment)

Add the following to the pre-commit config:

- repo: local
  hooks:
  - id: terraform_docs
    name: terraform_docs
    entry: ./hooks/terraform_docs.sh
    args:
    - --hook-config=--path-to-file=README.md
    - --hook-config=--use-standard-markers=false
    - --hook-config=--create-file-if-not-exist=true
    - --hook-config=--add-to-existing-file=true
    language: system
    files: (\.tf)$

Add a new file outputs.tf:

output "my_output" {
  value       = aws_api_gateway_stage.blah.invoke_url
  description = "Description of the test output"
}

Stage the pre-commit config and the new outputs.tf file: git add .pre-commit-config.yaml outputs.tf

Using the existing README.md as a target:

  • Test default behaviour has not changed:
    • pre-commit run -a should add the terraform docs between the existing markers
    • Delete the README. Then pre-commit run -a should create the file as normal
    • Delete the contents of the README. Then pre-commit run -a should add the terraform docs as normal
  • Test the behaviour with "standard" markers
    • update the pre-commit config to set --hook-config=--use_standard_markers=true, and git add -u
    • Delete the README. Then pre-commit run -a should create the file as normal, but with different markers
    • git add README.md
    • update using terraform-docs: terraform-docs markdown --output-file=README.md .
    • there should be no changes made by terraform docs (git diff)
    • repeat this process in reverse (terraform-docs, then pre-commit)

For completeness, repeat the standard markers test for insertion between existing markers, and appending to an existing file with no markers (or check by code inspection).

Copy link
Collaborator

@yermulnik yermulnik left a comment

Choose a reason for hiding this comment

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

While code in hooks/terraform_docs.sh looks decent per se, the PR looks to be a mess of superfluous files local to contributor 🤔

set-up for test with no README

remove lock file - not needed

set-up to test adding to existing README

restore README to original
WIP struggling with bash variable scope

refactor perl command to allow variable expansion

working hook, needs tidy up
@jonmcewen jonmcewen force-pushed the terraform-docs-default-markers branch from df49658 to fd91685 Compare December 20, 2023 16:26
@jonmcewen
Copy link
Contributor Author

While code in hooks/terraform_docs.sh looks decent per se, the PR looks to be a mess of superfluous files local to contributor 🤔

Yep, it's still WIP. Better now, but needs a little tidy up before review. I will hopefully finish this tomorrow

@MaxymVlasov MaxymVlasov added feature New feature or request hook/terraform_docs Bash hook labels Dec 20, 2023
@jonmcewen jonmcewen changed the title Terraform docs default markers feat: Terraform docs default markers Dec 21, 2023
@jonmcewen jonmcewen marked this pull request as ready for review December 21, 2023 09:35
@jonmcewen
Copy link
Contributor Author

@MaxymVlasov this is ready for review now. Thank you

hooks/terraform_docs.sh Outdated Show resolved Hide resolved
hooks/terraform_docs.sh Outdated Show resolved Hide resolved
hooks/terraform_docs.sh Outdated Show resolved Hide resolved
hooks/terraform_docs.sh Outdated Show resolved Hide resolved
hooks/terraform_docs.sh Outdated Show resolved Hide resolved
hooks/terraform_docs.sh Show resolved Hide resolved
jonmcewen and others added 2 commits December 21, 2023 12:21
Co-authored-by: George L. Yermulnik <[email protected]>
@jonmcewen
Copy link
Contributor Author

@yermulnik @MaxymVlasov thanks for the review. Please take another look

Copy link
Collaborator

@yermulnik yermulnik left a comment

Choose a reason for hiding this comment

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

LGTM

@yermulnik
Copy link
Collaborator

Leaving this to @MaxymVlasov to also review and merge if he's fine with the change.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@MaxymVlasov MaxymVlasov left a comment

Choose a reason for hiding this comment

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

Great, we will switch to tf-docs default markers in v2.0

@MaxymVlasov MaxymVlasov changed the title feat: Terraform docs default markers feat(terraform_docs): Add tf-docs default markers support and describe how to migrate to them Dec 21, 2023
@MaxymVlasov MaxymVlasov changed the title feat(terraform_docs): Add tf-docs default markers support and describe how to migrate to them feat(terraform_docs): Add terraform-docs default markers support and describe how to migrate to them Dec 21, 2023
README.md Outdated Show resolved Hide resolved
@MaxymVlasov MaxymVlasov merged commit 4a0e1fe into antonbabenko:master Dec 21, 2023
5 checks passed
antonbabenko pushed a commit that referenced this pull request Dec 21, 2023
# [1.86.0](v1.85.0...v1.86.0) (2023-12-21)

### Features

* **`terraform_docs`:** Add `terraform-docs` default markers support and describe how to migrate to them ([#609](#609)) ([4a0e1fe](4a0e1fe))
@antonbabenko
Copy link
Owner

This PR is included in version 1.86.0 🎉

@jonmcewen
Copy link
Contributor Author

Thanks for a great contribution experience everyone!

@jonmcewen
Copy link
Contributor Author

I just found this: https://github.com/terraform-docs/terraform-docs/#pre-commit-hook

Can you offer any comparison between the two @antonbabenko ?

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Dec 22, 2023

Wow, didn't know that they have hooks.

As far as I see, https://github.com/terraform-docs/terraform-docs/#pre-commit-hook was added 2021-05-19 in 0.12.0, when terraform_docs hook exist from 2018-05-16 (1.7.0)

First of all, example in README

repos:
  - repo: https://github.com/terraform-docs/terraform-docs
    rev: "v0.17.0"
    hooks:
      - id: terraform-docs-go
        args: ["markdown", "table", "--output-file", "README.md", "./mymodule/path"]

just not works, if, ofc, you want to add

      - id: terraform-docs-go
        args: ["markdown", "table", "--output-file", "README.md", "./mymodule/path"]
      - id: terraform-docs-go
        args: ["markdown", "table", "--output-file", "README.md", "./mymodule/path2"]
     ....
      - id: terraform-docs-go
        args: ["markdown", "table", "--output-file", "README.md", "./mymodule/pathN"]

And I don't get how to make work

repos:
  - repo: https://github.com/terraform-docs/terraform-docs
    rev: "v0.17.0"
    hooks:
      - id: terraform-docs-go

with default .terraform-docs.yml (with formatter: "markdown") -

image

So far, I can say that terraform-docs/terraform-docs hooks just not work in real-case scenarios

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request hook/terraform_docs Bash hook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants