-
Notifications
You must be signed in to change notification settings - Fork 32
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
Sync with template repository to add pre-commit checks and CI #68
Conversation
This adds a new pre-commit checker (`pre-commit install` to set up locally)
This was done automatically with `pre-commit run --all-files`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. It looks good to me in general -- I only asked a couple of minor questions.
run: make all | ||
env: | ||
VERSION: v${{ github.event.inputs.version }} | ||
REVMARK: ${{ github.event.inputs.revision_mark }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part, which pre-dates the PR, make me a little sad because the CI build that ends in the artifacts has the following version string in the title page: Version v, 2024-01-27:
. In other words, VERSION and REVMARK end up being empty here. Is it possible to fix this somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure we can get a default from the the makefile. I'll look into this today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is unrelated to this PR I've file #72 and plan to address this separately.
Otherwise pre-commit will complain.
Now includes pre-commit instructions as well as a minor workflow update.
In the future this will make it easier to add more pre-commit checks to ensure consistency.