mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-01 02:02:20 +00:00
fix(hooks): address Greptile review on PR #28703
Resolves two findings from the automated code review: 1. CONTRIBUTING.md: shrink the new Conventional Commits / Branches section to a 2-line pointer at docs.litellm.ai. Per the team convention, the full documentation lives in the litellm-docs repo — see BerriAI/litellm-docs#208 for the companion change that adds the section to docs/extras/contributing_code.md. 2. .githooks/commit-msg: tighten the subject regex to also reject an uppercase first letter in the description. CI's subjectPattern is ^(?![A-Z]).+$ so the previous local hook would accept 'feat: Add thing' which would then fail the PR-title check. The local hook is now the strictly tighter of the two gates. Test cases extended to cover both the new rejection and the digit/symbol-start cases that remain allowed. Resolves LIT-3306 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1cef88ffe5
commit
8f0b29b015
3 changed files with 33 additions and 60 deletions
|
|
@ -43,7 +43,11 @@ case "$subject" in
|
|||
esac
|
||||
|
||||
ALLOWED_TYPES="feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert"
|
||||
PATTERN="^(${ALLOWED_TYPES})(\([^)]+\))?!?: .+"
|
||||
# Description must not start with an uppercase letter — kept in sync with the
|
||||
# subjectPattern in .github/workflows/conventional-commits.yml so the local
|
||||
# hook is the strictly tighter of the two gates. (Without this guard, a commit
|
||||
# like "feat: Add thing" passes locally but fails the PR-title CI check.)
|
||||
PATTERN="^(${ALLOWED_TYPES})(\([^)]+\))?!?: [^A-Z].*"
|
||||
|
||||
if printf '%s' "$subject" | grep -Eq "$PATTERN"; then
|
||||
exit 0
|
||||
|
|
@ -55,6 +59,7 @@ cat >&2 <<EOF
|
|||
Got: $subject
|
||||
|
||||
Expected: <type>(<scope>)!: <description>
|
||||
(description must start with a lowercase letter)
|
||||
|
||||
Allowed types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
|
||||
Examples:
|
||||
|
|
|
|||
|
|
@ -55,65 +55,7 @@ That's it! Your local development environment is ready.
|
|||
|
||||
## Commit and Branch Conventions
|
||||
|
||||
LiteLLM enforces two community specs:
|
||||
|
||||
- **Commits** follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/) — `<type>(<scope>)!: <description>`
|
||||
- **Branches** follow [Conventional Branches](https://conventional-branch.github.io/) — `<type>/<description>`
|
||||
|
||||
### Commit message format
|
||||
|
||||
```
|
||||
<type>(<optional scope>)!: <description>
|
||||
|
||||
<optional body>
|
||||
|
||||
<optional footer>
|
||||
```
|
||||
|
||||
Allowed `<type>` values: `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert`. Add `!` before `:` for breaking changes.
|
||||
|
||||
Examples:
|
||||
```
|
||||
feat(router): add weighted round-robin strategy
|
||||
fix(bedrock): decouple STS region from aws_region_name
|
||||
chore(deps): bump black to 26.3.1
|
||||
refactor!: drop Python 3.8 support
|
||||
```
|
||||
|
||||
PR titles must follow the same format — squash-merge uses the PR title as the commit subject, and a CI check validates it.
|
||||
|
||||
### Branch naming
|
||||
|
||||
Format: `<type>/<short-description>` where `<type>` is one of `feature`, `bugfix`, `hotfix`, `release`, `chore`.
|
||||
|
||||
Examples:
|
||||
```
|
||||
feature/weighted-round-robin
|
||||
bugfix/streaming-empty-chunks
|
||||
chore/bump-black
|
||||
hotfix/auth-bypass
|
||||
```
|
||||
|
||||
Branches always allowed (bypass the check): `main`, `litellm_internal_staging`, `dependabot/*`, `gh-readonly-queue/*`.
|
||||
|
||||
### Installing the hooks
|
||||
|
||||
The hooks live in `.githooks/` and are opt-in. Run once per clone:
|
||||
|
||||
```bash
|
||||
make install-hooks
|
||||
```
|
||||
|
||||
This sets `core.hooksPath=.githooks` for the local repository. The hooks run on `git commit` (subject validation) and `git push` (branch validation).
|
||||
|
||||
In a rare emergency you can bypass them per-command:
|
||||
|
||||
```bash
|
||||
git commit --no-verify -m "..."
|
||||
git push --no-verify
|
||||
```
|
||||
|
||||
To uninstall: `git config --unset core.hooksPath`.
|
||||
Commits follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) and branches follow [Conventional Branches](https://conventional-branch.github.io/). Run `make install-hooks` once per clone to enable the local git hooks that enforce these — see the [contributor docs](https://docs.litellm.ai/docs/extras/contributing_code#commit-and-branch-conventions) for the full type list, examples, the protected-branch bypass list, and how to opt out.
|
||||
|
||||
### 2. Development Workflow
|
||||
|
||||
|
|
|
|||
|
|
@ -104,6 +104,13 @@ def test_commit_msg_accepts_conventional_subjects(tmp_path, subject):
|
|||
"ux: thing", # unknown type
|
||||
"Feat(router): capital type", # types are lowercase
|
||||
"feat(router):", # empty description
|
||||
# Description must start with a lowercase letter — kept in sync with
|
||||
# the CI workflow's subjectPattern so the local hook never accepts a
|
||||
# subject that CI will later reject.
|
||||
"feat: Add thing",
|
||||
"fix(router): Decouple something",
|
||||
"chore: BUMP deps",
|
||||
"feat: A",
|
||||
],
|
||||
)
|
||||
def test_commit_msg_rejects_invalid_subjects(tmp_path, subject):
|
||||
|
|
@ -115,6 +122,25 @@ def test_commit_msg_rejects_invalid_subjects(tmp_path, subject):
|
|||
assert "Conventional Commits" in result.stderr
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"subject",
|
||||
[
|
||||
# Lowercase letter — the common case.
|
||||
"feat: lowercase start is fine",
|
||||
# The CI's `^(?![A-Z]).+$` rejects only uppercase A-Z, so digits and
|
||||
# symbols are still allowed; mirror that behavior here.
|
||||
"feat: 1-based indexing now works",
|
||||
"fix(deps): @types/node bump",
|
||||
],
|
||||
)
|
||||
def test_commit_msg_accepts_non_uppercase_starts(tmp_path, subject):
|
||||
result = _run_commit_msg(subject, tmp_path)
|
||||
assert result.returncode == 0, (
|
||||
f"hook rejected a valid non-uppercase-start subject:\n"
|
||||
f" subject: {subject!r}\n stderr: {result.stderr}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"subject",
|
||||
[
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue