Skip to content

precommit: terraform fmt makes changes on disk - #1

Merged
topher200 merged 1 commit into
masterfrom
topher200/patch-0.1.17
Jan 27, 2022
Merged

topher200 merged 1 commit into
masterfrom
topher200/patch-0.1.17

Conversation

@topher200

@topher200 topher200 commented Jan 27, 2022 •

Copy link
Copy Markdown
Collaborator

Issues Addressed

Most of our pre-commit checks just make the changes that are rote, obvious
changes. Not terraform fmt! Instead, it just reports the error and exits,
meaning you have to fix the issue manually. And there's no reason for it, it's
just someone decided to do it that way and the maintainers won't switch it
back. Comment thread: gruntwork-io#48

Summary of Changes

I patched precommit to use the expected behavior. I included a shell
file to add the patch so it's easy to stay up-to-date with upstream.

Test Plan

  • Tested that precommit now automatically updates terraform files when
    using this branch for precommit.

 ### Issues Addressed
Most of our pre-commit checks just make the changes that are rote,
obvious changes. Not `terraform fmt`! And there's no reason for it, it's
just someone decided to do it that way and the maintainers won't switch
it back. Comment thread:
gruntwork-io#48

 ### Summary of Changes
I patched precommit to use the expected behavior. I included a shell
file to add the patch so it's easy to stay up-to-date with upstream.

 ### Test Plan
- Tested that precommit now automatically updates terraform files when
  using this branch for precommit.
@topher200
topher200 requested a review from vlindhol January 27, 2022 06:22
@vlindhol

Copy link
Copy Markdown

So just to recap: terraform fmt still makes changes to disk, but the pre-commit hook started using terraform fmt -check -diff which doesn't?

@topher200

Copy link
Copy Markdown
Collaborator Author

@vlindhol that's exactly correct!

@topher200
topher200 merged commit 985d1d4 into master Jan 27, 2022
@topher200
topher200 deleted the topher200/patch-0.1.17 branch January 27, 2022 16:14
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.

2 participants