Enable all pre-commit hooks in ci

[4ydan]

Task

Instead of invoking each hook individually we could just run pre-commit run -a. We need to:

• [ ] fix sqlfluff errors/warnings
• [ ] fix hanging of "Fix Lint groovy findings" (maybe use https://github.com/nice-move/prettier-plugin-groovy instead?)
• [ ] install packages for the hook fix-npm-groovy-lint
• [ ] remove cargo-fmt stage to not perform this check twice
• [ ] remove npm linting step inside frontend test stage to not perform this check twice
• [ ] install packages for eslint prettier and fmt hook

This would make the pipeline a bit more transparent, as the stage frontend test does also npm format:check, npm linting and npm test. And auto include new checks to the pipeline without needing to manually edit the Jenkinsfile.

[markus2330]

Thank you for reporting this improvement!

we could just run pre-commit run -a.

I fully agree that this is the goal here.

This would make the pipeline a bit more transparent

:+1:

[markus2330]

@4ydan I think lots of progress happened here? Can you check in the top post what is already done?

[4ydan]

Updated, sadly the other stuff is not that trivial. We would need to use a dockerfile that has all the backend and frontend stuff needed for the tasks.

[markus2330]

As long as there isn't something that is not checked in the CI pipeline at all (but is checked in pre-commit), I don't worry.

[4ydan]

What do we think about the idea to have a pre-commit file in each subdirectory? One in backend One in frontend One in e2e One in ci (groovy lint)

This would make the pre-commit file also more readable and enable us to easier integrate it into CI, not needing to build a docker image with the full stack.

[markus2330]

This sounds like a great idea. So the CI would execute the individual checks in each directory (with different Dockerfiles) but for devs it is basically unchanged?

[4ydan]

I hope so, I have to test if it works like I would imagine it to work. Maybe the first 11 lines are duplicates

# https://pre-commit.com/
repos:
  # General hooks
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
      - id: check-merge-conflict
      - id: end-of-file-fixer
      - id: mixed-line-ending
      - id: trailing-whitespace
        args: ["--markdown-linebreak-ext=md"]

But else it should hopefully still execute all necessary checks.

Only change for devs is to write new checks in the according .pre-commit-config.yaml And it would be harder to setup all pre-commit hooks I guess, not sure how good that is.

[4ydan]

Another solution seems to be to use the makefile to create different pre-commit targets. We could create backend, frontend, etc..

[markus2330]

Keeping Makefiles and pre-commit separate sounds better. pre-commit already provides a CLI interface, so I don't see a big benefit in having the targets available in Makefiles, additionally. Or do I miss something important here?

[4ydan]

Yea, I dont like the Makefile idea too much too. If we are fine increasing the setup complexity, it would be the best splitting the pre-commit file at least in two (one in the backend and one in frontend) Full stack devs would need to install both, but other than that, it would resolve some of the complexity and enable people to start quicker developing with a subproject when everything that is needed in that subfolder is where it is.

[4ydan]

I just realized this would only work if the subfolders are git submodules or git repos. So we cant do this, as multiple installations of pre-commits are not possible, they overwrite each other.

[markus2330]

Let us rest this issue again a bit. Let us pick up again when something changes in pre-commit.

[markus2330]

Currently sql format fails again and Fix Lint groovy findings hangs. So we should get this fixed in CI, to avoid formatting errors creeping in again.

I added (unticked) to top post:

• [ ] fix sqlfluff errors/warnings
• [ ] fix hanging of "Fix Lint groovy findings" (maybe use https://github.com/nice-move/prettier-plugin-groovy instead?)