Add for support Git hooks

[mrsimonemms]

See also an RFC (internal)

Projects such as Pre-Commit and Husky allow checks to be made locally prior to pushing to the repo. This is a good opportunity to run checks on the codebase, such as linting and ensuring styling of the code follows an established pattern.

If we used Pre-Commit, this would be useful starting point:

repos:
  - repo: https://github.com/mrsimonemms/pre-commit-hooks
    rev: v1.2.0
    hooks:
      - id: go-fmt-import
      - id: go-vet
      - id: gofumpt
      - id: go-err-check
      - id: go-static-check
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: pretty-format-json
        args:
          - --autofix
          - --no-sort-keys
      - id: check-json
      - id: check-yaml
        args:
          - --allow-multiple-documents
      - id: end-of-file-fixer
      - id: trailing-whitespace
      - id: end-of-file-fixer
      - id: trailing-whitespace
  - repo: https://github.com/trussworks/pre-commit-hooks
    rev: v1.1.1
    hooks:
      - id: markdown-toc
  - repo: https://github.com/DavidAnson/markdownlint-cli2
    rev: v0.13.0
    hooks:
      - id: markdownlint-cli2
  - repo: https://github.com/golangci/golangci-lint
    rev: v1.58.2
    hooks:
      - id: golangci-lint

This would also need a .markdownlint.json similar to this

{
  "MD007": {
    "indent": 2
  },
  "MD013": {
    "code_blocks": false,
    "tables": false
  },
  "MD032": false,
  "MD033": false,
  "default": true
}

This would also be worth putting in as a GitHub Action.

[patrickdappollonio]

Do you happen to know if any of these tools have a github action that could leverage caching? From personal experience, some of the pre-commit apps you can use can take a while to load. Case in point being golangci-lint which takes a hell of a long time running in GitHub actions.

Public repos having free GHA is fine, but for our private repos this could rack up compute times by a lot if caching is not leveraged!

[mrsimonemms]

Yes, see this thing I've used

[patrickdappollonio]

Awesome and good to know! I see the cache, I'm just not sure how it leverages it per-linter knowing there's no way that you can call a custom binary and have no idea where the cache for those binaries could land, but I trust you've tested it far more than I do 😄

The example you shared above calls out golangci-lint, I wrote a starting list of linters which also includes some of the ones from your own repo at the top, maybe we could merge those into the golangci-lint settings if need be.

I put the GHA for golangci-lint which caches every single linter enabled through the tool, so I can 100% confirm that it's not possible to break cache with it.

[mrsimonemms]

Yeah, the cache is for the dependencies pre-commit installs. One annoyance with pre-commit is that the initial installation can (not always) be slow - as a rule of thumb, the more dependencies the slower it is to install. However, once it's in the cache it's fairly rapid (< 2 seconds typically)

[patrickdappollonio]

I'm honestly less worried about installations and more about the execution cache itself.

GolangCI-lint takes over 6 minutes on a GHA runner the first time. Each run can cache previous results so if the code hasn't changed, those checks are skipped and the same outcome is returned.

Without "execution" caching we would be seeing larger and larger times of CI which would increase the feedback loop timer.

Between download cache and execution cache I wish we would have both, but if I had to choose, execution for me (especially for GolangCI-lint) is the most important.

[mrsimonemms]

I can only suggest doing a bit of experimentation on this. I've used GolangCI-Lint on all my projects and don't recall that being especially slow - it might be because it's only being run on the changed files in Pre-commit or it might be other reasons. But this is definitely something to keep in mind.

[patrickdappollonio]

I'm down to take over the RFC and this issue if you want me to run the experimentation and take it to completion, but I don't want to step in your own proposal. Since you're the RFC's driver, the goal is for the driver to provide the "burden of proof" that the situation they're looking for comments adapt to, well, the comments from the other team members.

Happy to go either way and change the RFC's driver to me though!