-
-
Notifications
You must be signed in to change notification settings - Fork 672
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
disabled logging now possible #69
Conversation
@@ -2,12 +2,14 @@ | |||
# See http://pre-commit.com/hooks.html for more hooks | |||
repos: | |||
- repo: git://github.com/antonbabenko/pre-commit-terraform | |||
rev: v1.7.0 | |||
rev: v1.7.1 | |||
sha: 091f8b15d7b458e5a0aca642483deb2205e7db02 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can run pre-commit autoupdate
to get it updated. sha
is not required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm maybe my pre-commit
is out of date. It was upset at me. Will update.
hooks: | ||
- id: terraform_fmt | ||
- id: terraform_docs | ||
# - id: terraform_docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you disable this hook it won't update documentation automatically. Not sure you want this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hook failed for me and I was somewhat surprised at what it was requiring. I've documented in the readme how I'm accustomed to running this tool, packing all the docs in main.tf
and having the tool generate the entire README.md
with inputs and outputs at the end. Requiring comments in the README and creating temp files in my root workspace that didn't get cleaned on failure had me think I'd rather file a bug against the hook repo and just run terraform-doc
manually for the moment.
Obviously it's your repo and project to do what you want with it but if it were mine, I'd probably try to run terraform-doc
as straightforwardly as possible, no surprises. That's just my take though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The terraform_docs
hook should not fail - this is the most important part. Please help me to figure out why it fails because I want people to use hook for real in real projects. (I tested it on Mac OS X and it seems to have some issues for some people on other OS). Maybe you can open an issue there.
The primary feature of pre-commit hooks is that developers who push PR or maintainers like you and me get them installed once and forget. The code is automatically formatted, the documentation is updated once a change is being committed - as a result, documentation is along with the code in the same PR.
It also simplifies manual steps like you have to do now during release.
I don't have strong objections if you prefer to do it another way, absolutely :)
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
No description provided.