Skip to content

feat: run linters and tests on each push, not just on open PRs - #1622

Closed
afn wants to merge 1 commit into
mainfrom
afn/run-tests-on-all-commits
Closed

feat: run linters and tests on each push, not just on open PRs#1622
afn wants to merge 1 commit into
mainfrom
afn/run-tests-on-all-commits

Conversation

@afn

@afn afn commented Dec 31, 2025

Copy link
Copy Markdown

Rename GitHub workflow from pr-validation to commit-validation, and run on all pushes to all branches, rather than only running for open PRs.

Concurrency is configured to cancel earlier builds on branches except for main.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 44 to +48
run: |
BASE=$(git merge-base ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }})
BASE=$(git merge-base ${{ github.sha }} ${{ github.sha }})
pnpx commitlint \
--from "$BASE" \
--to ${{ github.event.pull_request.head.sha }}
--to ${{ github.sha }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Lint range collapses to a single commit on push

Because BASE is computed as git merge-base ${{ github.sha }} ${{ github.sha }}, the base always equals the current SHA, so --from and --to are identical. On a push that contains multiple commits (e.g., after a rebase or when pushing a series of local commits), this configuration only lints the tip commit and skips earlier commits, allowing invalid commit messages through. If the intent is to validate all commits introduced by the push, use the previous head (e.g., github.event.before) as the start of the range.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

👍 That's legit. Marking this as do-not-merge for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

This compares github.sha with itself, so git merge-base will return the same commit.

@caio-pizzol caio-pizzol left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea to shift to push-based validation (note: this will increase CI usage - which I think is OK in this case)

Comment on lines 44 to +48
run: |
BASE=$(git merge-base ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }})
BASE=$(git merge-base ${{ github.sha }} ${{ github.sha }})
pnpx commitlint \
--from "$BASE" \
--to ${{ github.event.pull_request.head.sha }}
--to ${{ github.sha }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

This compares github.sha with itself, so git merge-base will return the same commit.

@harbournick
harbournick deleted the afn/run-tests-on-all-commits branch April 25, 2026 00:59
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.

3 participants