ci: auto lint/format fix in CI + test with bad formatting#172
ci: auto lint/format fix in CI + test with bad formatting#172
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
There was a problem hiding this comment.
Pull request overview
This PR adds automated linting and formatting fixes to the CI test workflow. When pull requests are created, the workflow will automatically run oxlint --fix and oxfmt --write, commit any fixes, and push them back to the PR branch before running tests.
Changes:
- Added
oxlint --fixandoxfmt --writecommands to auto-fix code style issues - Added auto-commit step that commits and pushes formatting fixes back to the PR branch
- Added
contents: writepermission to allow the workflow to push commits - Renamed job from
run_comfy_prtocomfy_pr_testfor clarity
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| contents: write | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
The PR description mentions adding "intentionally bad formatting in lib/slack/index.ts to verify the auto-fix works", but this file is not included in the changes. To properly test the auto-fix functionality, you should include a file with formatting issues in the PR. Without this, the CI workflow changes cannot be verified to work as intended.
| contents: write | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
The checkout action needs to be configured to checkout the PR branch with proper ref when running on pull_request events. By default, actions/checkout@v4 checks out a merge commit in pull_request context, which prevents pushing back to the PR branch. You should add ref: ${{ github.head_ref }} to the checkout step to ensure the actual PR branch is checked out, allowing the auto-commit to work properly.
| - run: bunx oxlint --fix | ||
| - run: bunx oxfmt --write | ||
| - name: Commit lint/format fixes | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| if ! git diff --quiet; then | ||
| git add -A |
There was a problem hiding this comment.
The git push command will fail when trying to push to a PR from a forked repository because workflows triggered by pull_request events from forks have restricted permissions and cannot push to the fork. This is a GitHub security feature. Consider using a token-based push mechanism or only enabling this auto-fix feature for PRs from branches in the same repository. You should add a condition to check if the PR is from a fork before attempting the push.
| - name: Commit lint/format fixes | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| if ! git diff --quiet; then | ||
| git add -A | ||
| git commit -m "style: auto-fix lint and formatting" | ||
| git push |
There was a problem hiding this comment.
Running the auto-fix before tests creates a potential issue where tests will run against code that hasn't been fixed yet if there are no changes, or against fixed code if there are changes. This could lead to inconsistent test results. Consider either: 1) Running tests both before and after auto-fix to ensure they pass in both states, 2) Moving the auto-fix to a separate job that runs after tests pass, or 3) Failing the workflow if fixes are needed and requiring developers to run the linter locally before pushing.
| - name: Commit lint/format fixes | |
| run: | | |
| git config user.name "github-actions[bot]" | |
| git config user.email "github-actions[bot]@users.noreply.github.com" | |
| if ! git diff --quiet; then | |
| git add -A | |
| git commit -m "style: auto-fix lint and formatting" | |
| git push | |
| - name: Check lint/format fixes | |
| run: | | |
| if ! git diff --quiet; then | |
| echo "Lint/formatting issues detected." | |
| echo "Please run 'bunx oxlint --fix' and 'bunx oxfmt --write' locally and commit the changes before pushing." | |
| exit 1 |
| - run: bunx oxlint --fix | ||
| - run: bunx oxfmt --write | ||
| - name: Commit lint/format fixes | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| if ! git diff --quiet; then | ||
| git add -A | ||
| git commit -m "style: auto-fix lint and formatting" |
There was a problem hiding this comment.
When the auto-fix commits changes and pushes to the PR branch, this will trigger a new pull_request synchronize event, which could re-run this workflow. While the second run won't find any changes to commit (since the code is already fixed), this causes unnecessary CI runs. Consider adding a condition to skip the workflow when the latest commit author is 'github-actions[bot]', or use [skip ci] in the commit message to prevent re-triggering the workflow.
f1fda78 to
7a98c87
Compare
Summary
oxlint --fixandoxfmt --writeto CI, auto-committing any fixes back to the branchlib/slack/index.tsto verify the auto-fix worksTest plan