Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add CI tests for the pre-commit hooks #9

Merged
merged 4 commits into from
Jul 1, 2024
Merged

Conversation

amchiclet
Copy link
Contributor

@amchiclet amchiclet commented Jul 1, 2024

This PR adds a pre-commit hook config. The new CI workflow installs the hook and then runs it.

The workflow just fakes a change git mv tests/python_simple.py python_simple.py, so pre-commit can pick it up.

Currently points to 1.76.0 which is the newest available tag at the moment.

For each new release, the hook should be updated after updating the release tag. Could probably be automated somewhat, but going to do that manually for now.

@amchiclet amchiclet marked this pull request as ready for review July 1, 2024 21:22
@amchiclet amchiclet requested a review from kopecs July 1, 2024 21:22
Copy link
Collaborator

@kopecs kopecs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving since this seems better than nothing, but I am not sure of the approach (for instance, I don't love doing it in the same git repo; I would prefer making a fresh one as part of the test).

I would probably also prefer us to have a script or Makefile which CI uses instead of scripting it directly there.

.github/workflows/test-pre-commit.yml Outdated Show resolved Hide resolved
Comment on lines +7 to +8
- repo: https://github.com/semgrep/pre-commit
rev: 'v1.76.0'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- repo: https://github.com/semgrep/pre-commit
rev: 'v1.76.0'
- repo: local

Can we just make this a local hook so it is testing the current configuration, rather than being a pointer to potentially something else?

See https://pre-commit.com/#repository-local-hooks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I can't just swap in local and there's more config fields that I need to understand. I'm gonna give up on trying that for now.

Copy link
Collaborator

@kopecs kopecs Jul 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may not be supported for the Python ones. Not sure.

ETA: Oh I see. Yeah, I don't like this, but that's more complex than I thought.

@amchiclet amchiclet merged commit d949bd0 into develop Jul 1, 2024
2 checks passed
@kopecs kopecs mentioned this pull request Jul 2, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants