feat: add automatic linting suggestion github workflow#46
Conversation
|
Oh wow, this is fantastic! I have never heard reviewdog before but this is fantastic! Do this report problems for files that doesn't get auto-formatted? (e.g. markdown files) |
|
For what this does, there is surprisingly little configuration we'd have to maintain. The reviewdog project looks actively maintained as well. I'm very much in favor of moving forward with this. #34 is merged now. I think this PR needs to be rebased on top of that to get this PR merged as well. @linusboehm are there any drawbacks to moving ahead with this PR that we're not seeing? It looks like only upside to me. |
| permissions: | ||
| contents: read | ||
| checks: write | ||
| issues: write | ||
| pull-requests: write |
There was a problem hiding this comment.
Will need to move this as well, I don't think the default actions permission allows writes.
There was a problem hiding this comment.
You mean move it to .github/workflows/pre-commit.yml? Are the permissions set for that workflow already?
There was a problem hiding this comment.
Ohno! Sorry I didn't reply to this comment before merging, all the permissions are needed in pre-commit.yml.
There was a problem hiding this comment.
Again sorry for not being clear.
The only time I've tried to use this was in a codebase with the requirement that only changed lines are formatted (instead of whole files) as the codebase didn't have consistent formatting. That seemed to be a bit cumbersome and sometimes lead to formatting changes that obfuscated the underlying code change. Given that beman repos should be formatted consistently according to the format spec, this shouldn't be a problem and I can't think of any other reasons not to give it a try. I'll rebase and address the review comments soon (@wusatosi thanks for the review). |
d00c82b to
581b5e5
Compare
581b5e5 to
a7e1a0f
Compare
| with: | ||
| python-version: 3.13 | ||
|
|
||
| - name: Cache pre-commit hooks |
There was a problem hiding this comment.
This comes with pre-commit/action and is actually not needed.
There was a problem hiding this comment.
How does it set the paths to cache when not explicitly specifying them?
There was a problem hiding this comment.
pre-commit/action caches based on the default configuration location.
Usually these actions come with appropriate caching built-in (not the pip install pre-commit, but oh well).
e.g. see action log:
https://github.com/beman-project/exemplar/actions/runs/11385583999/job/31675772106#step:6:1
I guess leaving this in is fine, the restored cache by this step will be overwritten/ merged in with that from pre-commit/action step so really no harm is done. ¯\_(ツ)_/¯
| - uses: pre-commit/[email protected] | ||
| with: | ||
| extra_args: --files ${{ steps.changed-files.outputs.all_changed_files }} | ||
| continue-on-error: true |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
This is in-fact needed. I am wrong.
|
Ops didn't realize this is already merged |
this commit adds a github workflow to automatically run
pre-commitand suggests changes to a PR, to fix any linting errors. An example error + suggested fix can be seen here: linusboehm#2Suggestions can be accepted + committed from the PR on github.