terraform-fmt hook behaviour change #48
Comments
|
A PR to support some sort of flag that enables the old behavior (overwriting the files) is welcome! |
|
@brikis98 do you think the following can solve the problem rather than a flag: |
|
I would agree the changes to format in v0.1.12 don't make sense. If I wanted to know all the errors I would have run lint or validate on the files first. IMHO running format should just auto-correcting issues as the formatter deems fit. It should fail on invalid syntax or logic issues immediately so they can be fixed and need to be re-run, hence why you should validate first if you need to. I don't care about issues it finds regarding spacing, etc that is why I am using the auto-formatter. Also the diff may matter from the perspective of debugging a CI or a dry-run perspective, but for the default it doesn't. Changes should happen immediately on disk. I would propose the default behaviour be restored and if you don't want changes made on disk a |
|
+1 for restoring previous functionality |
|
Yeah at the moment the way to go is to use v0.1.11 in the meantime. |
|
@brikis98 I filed a PR that allows for both options via a flag ( Obviously we can invert the logic pretty easily if needed, but I'd like to hear your thoughts first! |
### 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.
### 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.
|
is this going to be released any time soon? 0.1.17 still does not actually modify the files? |
|
Is there any further thoughts on this behavior being added even as an optional argument? |
Previously the terraform-fmt hook made changes to the files, whereas now it only shows the difference and errors out. I did some digging and found that it was an intentional change as part of this PR: #46
It would be nice to have the old behaviour as an option. Generally my terraform files are syntactically valid, but incorrectly formatted, the previous behaviour took away the pain point of needing to run
terraform fmtafter every change. The differences don't need to be analysed, so there's no need for the--diff --check.Thanks for providing these hooks, they've been super useful!
The text was updated successfully, but these errors were encountered: