chore(format): added pre-commit hooks and formatted files

[aeswibon]

Description

• Adds pre-commit hooks to format files
• Fixes formatting along the codebase

Type of change

• Chore (changes which are not directly related to any business logic)

What's Changed

• Add a list of tasks or features here...

[vercel]

@aeswibon is attempting to deploy a commit to the Hatchet Team on Vercel.

A member of the Team first needs to authorize it.

[aeswibon]

@abelanger5 Can you check the PR?

[abelanger5]

I'm going to request some other reviews on this as it's quite a large PR, but generally I think we want to ignore auto-generated files (like migration.sql and auto-generated REST clients). Will do a deeper dive later today.

[steebchen]

This looks good but indeed as @abelanger5 said the prisma .sql migration files under prisma/migrations can't be changed now as it will break the migrations. We can only change this with a major release where migrations would need to be reset. Ignoring auto-generated clients makes sense as well. If you need any guidance on what else needs to be ignored I'm happy to help and/or make a list.

[aeswibon]

So like, I should just ignore all the files in prisma/migrations folder. Anything else?

[steebchen]

Yes, pretty much. Also, the linter needs to run as the last item of the each task where something is generated. I think running the linter on generated files is fine from my perspective as that ensures consistent formatting throughout the project. Once you have that the generate CI job should succeed (and the lint seems to be a leftover, so not related to this PR).

[steebchen]

LGTM! Please merge with incoming changes and then just run the linter again and it should be fine. Also check that the checks all pass (except for golangci-lint that is expected to fail in this case). I'll check with @abelanger5 if there is anything else before we merge.

[aeswibon]

Sure, I will resolve all the conflicts.

[steebchen]

@aeswibon can you please look into the CI failures? looks like the generate task fails

[aeswibon]

Sure, I will check that. And will also resolve the merge conflicts

[aeswibon]

@steebchen I have cleared the merge conflict and fixed the pipeline

[grutt]

@steebchen I'd like to give this another look before merging since it's a fairly large commit. Feel free to review and I'll look in the am.

[steebchen]

@aeswibon thanks, one more step would be to run the generate step and hopefully then it will match exactly and the generate CI step will succeed as well
@grutt it looks good from my side, I'll be waiting for your confirmation

[steebchen]

@aeswibon Can you please run the generate script again and push. You can see in the generate CI step that there are still some mismatches. If you run generate-all and make sure the linter ran and push, it should match. And then I can merge this which would be nice. Thanks!

[aeswibon]

Sure

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
hatchet-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 11, 2024 2:12pm

[steebchen]

@aeswibon generate step is still failing unfortunately, also please merge from main again

[aeswibon]

@steebchen Generate step would fail as in the pipeline, python files are also generated, due to which pre-commit would fail.

[steebchen]

@aeswibon I don't follow, if you run generate and run the lint, and then push the changes, the CI step should result in the same files and then succeed? Can you elaborate? And the merge is also required otherwise I can't merge due to the conflicts

[aeswibon]

@steebchen I have resolved the conflict. Also, when I saw the logs, the files were re-generated using openapi-generator

[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/hatchet_sdk/clients/rest/models/api_error.py
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/docs/APIError.md
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/hatchet_sdk/clients/rest/models/api_errors.py
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/docs/APIErrors.md
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/hatchet_sdk/clients/rest/models/api_meta.py
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/docs/APIMeta.md
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/hatchet_sdk/clients/rest/models/api_meta_auth.py
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/docs/APIMetaAuth.md
[main] INFO o.o.codegen.TemplateManager - writing file /home/runner/work/hatchet/hatchet/./python-sdk/tmp/hatchet_sdk/clients/rest/models/api_meta_integration.py

I have previously tried to run the generator for python files and then tried lining but the pipeline failed.

[steebchen]

Ah I understand the problem now. I'm new to that pre-commit CLI so I'm not sure the best way to fix it.

What command are you running locally to run the format?

I think we can just run that instead of what we run right now in CI, so that it just formats the files instead of failing. We have a CI check for the diff anyway, so this ensures that everything is consistent.

[aeswibon]

Ah I understand the problem now. I'm new to that pre-commit CLI so I'm not sure the best way to fix it.

What command are you running locally to run the format?

I think we can just run that instead of what we run right now in CI, so that it just formats the files instead of failing. We have a CI check for the diff anyway, so this ensures that everything is consistent.

For the command, I have added it in the Taskfile.yml as pre-commit-run

[steebchen]

Yeah but it fails in the CI. It doesn't seem to fail on your machine. What am I missing?

[aeswibon]

Yeah but it fails in the CI. It doesn't seem to fail on your machine. What am I missing?

It fails in the CI because the code is rewritten. Pre-commit will fail initially because the generation step would generate unformatted code. pre-commit will check the files and if the step fails then it formats the code.

[steebchen]

I understand why it fails, but does the command fail locally as well? I just want to figure out how to make it not fail for CI, as it is expected that it needs to re-format the code, because all generated code is newly generated in CI.

[aeswibon]

Locally, pre-commit would fail initially, then it would format the file after which it would pass.

[steebchen]

I see, got it. We would need to think of a way to make it work in CI... running it twice while ignoring the first result is an option although it's not optimal

[steebchen]

@aeswibon I removed ggshield as it seems to require an API key, and I changed the command to run twice basically so it doesn't fail.

If there is still a diff from the source code after this, the test / generate CI check will catch it as it manually checks for any diff.

So unless anyone has any other concerns I'll merge this as is

[steebchen]

THanks @aeswibon for the contribution! I will most likely split the python client into its own repository, but this helped a lot as we wanted to set up a linter for python anyway but didn't get to it.