Skip to content

Replace strict Conventional Commits PR-title check with a sanity check#50

Merged
Pawansingh3889 merged 1 commit into
mainfrom
chore/soften-pr-title-validator
May 9, 2026
Merged

Replace strict Conventional Commits PR-title check with a sanity check#50
Pawansingh3889 merged 1 commit into
mainfrom
chore/soften-pr-title-validator

Conversation

@Pawansingh3889
Copy link
Copy Markdown
Owner

The previous pr_title.yml workflow used amannn/action-semantic-pull-request@v5 to enforce strict Conventional Commits format on every PR title (feat:, fix:, etc.) plus a lowercase-subject regex. That conflicted with the project's actual title convention ("Add Wxxx rule-name (description)", "Fix Snnn false negatives ...") and so the validate check went red on every merged PR (e.g. #46, #47, #48, #49). Branch protection does not require this check, so the merge button stayed enabled and the failures shipped silently in the merged commits' status timelines.

Replacing the strict format enforcer with a small inline shell script that catches the actual problems we have hit in practice:

  1. Empty / whitespace-only titles -- noticeable in gh pr list.
  2. Auto-generated branch-name titles (e.g. "Feat/w024-select-distinct -suspicious") that GitHub picks when an async PR-template render wipes out a freshly typed title. Matches a regex of the form ^[A-Z][a-z]+/[a-z0-9-] which catches the real cases without touching anything that looks like a sentence.
  3. Titles longer than 100 characters, which truncate badly in the UI.

Project titles like "Add W024 select-distinct-suspicious (...)" pass the sanity check; titles like "Feat/foo-bar" fail. Verified the regex matrix locally before committing.

To restore stricter enforcement later, swap the inline script back for amannn/action-semantic-pull-request@v5 and adjust the project title convention to start with feat: / fix: prefixes.

What changed

Related issue

If this adds or changes a rule

  • Rule ID is the next unused one in its family (E00N / W0NN / S00N / P00N)
  • Rule class registered in sql_guard/rules/__init__.py
  • Fixture line in tests/fixtures/errors.sql or warnings.sql
  • "Fires on bad SQL" test in tests/test_rules.py
  • "Does NOT fire on safe SQL" test (using tmp_path)
  • README rule table + Key Numbers count updated
  • Rule is not a removal or rename of an existing rule (breaking change — see GOVERNANCE.md)

Tests

  • pytest -q passes
  • ruff check . passes

Checklist

  • One logical change per commit, conventional commit style (feat:, fix:, docs:, chore:, ci:, test:, style:)
  • CHANGELOG.md [Unreleased] entry if user-facing

Anything else reviewers should know

The previous pr_title.yml workflow used amannn/action-semantic-pull-request@v5
to enforce strict Conventional Commits format on every PR title (feat:,
fix:, etc.) plus a lowercase-subject regex. That conflicted with the
project's actual title convention ("Add Wxxx rule-name (description)",
"Fix Snnn false negatives ...") and so the validate check went red on
every merged PR (e.g. #46, #47, #48, #49). Branch protection does not
require this check, so the merge button stayed enabled and the failures
shipped silently in the merged commits' status timelines.

Replacing the strict format enforcer with a small inline shell script
that catches the actual problems we have hit in practice:

1. Empty / whitespace-only titles -- noticeable in `gh pr list`.
2. Auto-generated branch-name titles (e.g. "Feat/w024-select-distinct
   -suspicious") that GitHub picks when an async PR-template render
   wipes out a freshly typed title. Matches a regex of the form
   ^[A-Z][a-z]+/[a-z0-9-] which catches the real cases without
   touching anything that looks like a sentence.
3. Titles longer than 100 characters, which truncate badly in the UI.

Project titles like "Add W024 select-distinct-suspicious (...)" pass
the sanity check; titles like "Feat/foo-bar" fail. Verified the regex
matrix locally before committing.

To restore stricter enforcement later, swap the inline script back for
amannn/action-semantic-pull-request@v5 and adjust the project title
convention to start with feat: / fix: prefixes.
@Pawansingh3889 Pawansingh3889 merged commit 845b8bf into main May 9, 2026
6 of 8 checks passed
Pawansingh3889 added a commit that referenced this pull request May 9, 2026
…ook (#51)

The .pre-commit-config.yaml ran sql-sop with `--severity error` against
every `*.sql` file in the repo. The only SQL files currently in the
repo are the unit-test fixtures under tests/fixtures/ -- and those
fixtures deliberately contain bad SQL (E001 DELETE without WHERE, E002
DROP without IF EXISTS, E003 GRANT/REVOKE, E004 string concatenation in
WHERE, E005 INSERT without column list, E006 UPDATE without WHERE,
plus warning- and structural-rule fixtures) so the rule unit tests can
assert on those exact patterns.

Result: pre-commit.ci has been reporting "Found 6 issues (6 errors) in
1 file" against tests/fixtures/errors.sql on every single PR for
months. The pre-commit-ci status has been red the whole time but
hidden behind the also-always-red validate check (just fixed by #50);
on PRs that don't touch SQL fixtures, the dogfooding hook adds no
diagnostic value because the failures are by design.

Adding `exclude: ^tests/fixtures/` so the hook only runs against real
SQL files outside the fixtures tree. No real .sql files exist in the
repo today, so the hook is effectively dormant; the moment someone
adds a migration script or an example query outside tests/fixtures/
it kicks in. Verified locally that the exclude pattern matches all
four fixture files (clean.sql, contract_drift.sql, errors.sql,
warnings.sql).
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.

1 participant