Add pre commit

[27Bslash6]

tldr

Minimal QA Uplift and Automation

With reference to the previous PR, these changes introduce only one additional, optional (highly recommended) developer dependency: pre-commit, and offers guidance on its installation.

In doing so, we ensure that all standard linting and custom business logic QA is applied before PR review, including the existing yapf style choice, boilerplate, documentation, links and name length checks.

Rationale

Pre-commit

Git hook scripts are useful for identifying simple issues before submission to code review. We run our hooks on every commit to automatically point out issues in code such as missing semicolons, trailing whitespace, and debug statements. By pointing these issues out before code review, this allows a code reviewer to focus on the architecture of a change while not wasting time with trivial style nitpicks.

I believe the addition of this one tool will significantly improve DX and reduce cognitive load for both the developer and those responsible for code review. Reducing manual pre- and post-commit burden should also reduce the size of CONTRIBUTING.md as much of the QA will be implemented automatically, including Terraform and Python formatting, vulnerability checks, tdfoc creation etc, however I have elected to minimise the number of changes to CONTRIBUTING at this time, until code owners validate the proposed new developer flow.

Changes

No breaking changes are included in this PR. The use of the pre-commit hook is optional.

The only changes to the main codebase are aesthetic:

• fixing line-endings
• ensuring a blank newline at end of file
• convert tabs to spaces
• remove trailing whitespace

The check-documentation hook indicated that the compute-mig readme had missed changes, and so that file has been updated also.

A Makefile is included for convenience, but the use of which is optional.

Note: this will conflict with #680 so will rebase one or the other if accepted.

[27Bslash6]

I think this is an outstanding repo, and am very grateful for its availability. In particular the techniques for factory resource generation will be very useful for our organisation. Applying some standardised QA is my small way of trying to say thanks 🙏

[ludoo]

I'm a bit wary about relying on external code which we will then need to audit regularly, and would prefer coming up with our own static versions of pre commits

[ludoo]

I generally like the idea and thanks for taking the time to send this PR, but I see two issues that will require at least some internal discussion

• reliance on external code, which would need to be regularly audited
• increased friction for contributors who only work on Terraform and might not have Python or a working vitualenv installed

Let's use the comments to discuss these two points.

[juliocc]

+1 @ludoo's comments. I like the overall idea but we're introducing a ton of dependencies (pre-commit, random third-party plugins, etc). Some of the checks also break the standard conventions for certain code (e.g. the remove-tabs task is removing tabs from Go code).

That being said, I like the idea of a Makefile to simplify some tasks. @27Bslash6 @ludoo what do you think of a simple Makefile to run the existing tools (check, lint, tests etc) without introducing extra checks/tools?

[27Bslash6]

Thanks for taking the time to peruse, folks!

A few thoughts:

1. Pre-commit is optional, not mandatory. The addition of the YAML file doesn't do anything without developers first initialising the hook via something like brew install pre-commit && pre-commit init, at which point it kicks in automatically before each commit. No additional friction unless opted-in and though I strongly believe that, once adopted, you'll appreciate the benefits and find it hard to go back, you're free to continue doing it all manually.
2. The argument against adding third party dependencies is certainly valid. Though mitigated by usual open source conversation: exact version pinning and the maturity and health of the community-supported projects, it's admittedly the equivalent of adding new python modules, and a potential vector for bad actors. That said, all three non-Google projects are MIT licensed and pretty simple code, so bringing them in-house is eminently doable. Happy to do so if this is the preference.
3. I expect the Venn diagram of users who have Terraform and a recent version Python installed is pretty nearly a single circle, and if they're updating Terraform code don't they also need to update tfdocs via the Python script?
4. @juliocc good point re: go formatting. If you're interested in proceeding with this solution I'm very happy to push an update with exclude_types for the tab manipulation hook, and we might consider adding a gofmt hook in future.

[ludoo]

I'm going to close this, as it adds friction with no clear benefit. Feel free to reopen individual issues for each pre-commit feature and we can discuss there.