pre-commit
pre-commit copied to clipboard
gruntwork-io
terraform-fmt hook behaviour change
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: https://github.com/gruntwork-io/pre-commit/pull/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 fmt after 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!
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: terraform-fmt.sh can run terraform fmt terraform-diff.sh can run terraform fmt -diff -check
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 --dry-run flag be added to the arguments for the hook.
Yeah at the moment the way to go is to use v0.1.11 in the meantime. Maybe in the future I can try to implement @skoblenick approach if you guys agree.
@brikis98 I filed a PR that allows for both options via a flag (--no-autofix). I reverted the default behaviour to what used to be in place in v0.1.11 as I thought this is more in-line with other pre-commit hooks (e.g. from pre-commit/pre-commit-hooks: trailing-whitespace, sort-simple-yaml, end-of-file-fixer).
Obviously we can invert the logic pretty easily if needed, but I'd like to hear your thoughts first!
A PR to support some sort of flag that enables the old behavior (overwriting the files) is welcome!
@brikis98 do you think you can review my PR https://github.com/gruntwork-io/pre-commit/pull/53 ? Thanks!
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?
