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 ATX Header Support for terraform-docs #164

Merged

Conversation

michaelsaki
Copy link
Collaborator

@michaelsaki michaelsaki commented Jan 22, 2024

🗣 Description

This pull request:

  • Clones @mcdonnnj's forked branch from terraform-docs.
  • Builds and installs the binary from the clone.

💭 Motivation and context

This temporarily resolves #196 until @mcdonnnj's changes get merged into terraform-docs, at which point we can change back over to the standard of installing terraform-docs.

🧪 Testing

All automated tests passed. I also used act for local testing to make sure the workflow would behave like it should.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • If applicable, All future TODOs are captured in issues, which are referenced in the PR description.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • All new and existing tests pass.

✅ Pre-merge checklist

  • Revert dependencies to default branches.
  • Finalize version.

✅ Post-merge checklist

  • Create a release.

This is a temporary fix until @mcdonnnj has
his PR approved and merged into the terraform-docs
repo. This fix will perform a shallow clone of his
forked branch, build the binary, and install it.
@michaelsaki michaelsaki added documentation This issue or pull request improves or adds to documentation improvement This issue or pull request will add or improve functionality, maintainability, or ease of use labels Jan 22, 2024
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Some feedback for your consideration.

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

Some very minor things...

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
michaelsaki and others added 3 commits January 22, 2024 14:03
Co-authored-by: Nick <50747025+mcdonnnj@users.noreply.github.com>
Co-authored-by: Shane Frasier <jeremy.frasier@gwe.cisa.dhs.gov>
PATH is handled by `setup-go` so we can refactor the code setting it. Also we are taking advantage of the -C switch to handle building from the cloned repository.

Co-authored-by: Nick <50747025+mcdonnnj@users.noreply.github.com>
.github/workflows/build.yml Outdated Show resolved Hide resolved
`TODO` was placed on the wrong comment block. Also I am adding a link to the issue for the TODO.
Co-authored-by: dav3r <david.redmin@trio.dhs.gov>
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this! 👍

Can you please confirm that you re-tested this workflow after making the changes requested during the PR review?

@michaelsaki
Copy link
Collaborator Author

michaelsaki commented Jan 23, 2024

Thanks for taking care of this! 👍

Can you please confirm that you re-tested this workflow after making the changes requested during the PR review?

You are welcome! Yes, I ran any changes to the workflow in act, everything checked out locally.

Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

:shipit:

@mcdonnnj mcdonnnj added this pull request to the merge queue Mar 6, 2024
Merged via the queue into develop with commit 9020b55 Mar 6, 2024
5 checks passed
@mcdonnnj mcdonnnj deleted the improvement/install_atx_header_support_terraform-docs branch March 6, 2024 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This issue or pull request improves or adds to documentation improvement This issue or pull request will add or improve functionality, maintainability, or ease of use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a script to run terraform-docs and repair Markdown headers
4 participants