workflows: add new commit linter tool #11245
Conversation
Signed-off-by: Eduardo Silva <[email protected]>
edsiper
requested review from
celalettin1286,
cosmo0920,
niedbalski and
patrick-stephens
as code owners
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a commit-prefix linter: a new Python script that validates commit subjects, Signed-off-by lines, and infers expected prefixes from changed paths; a GitHub Actions workflow runs the checker on PRs and master pushes; documentation and unit tests accompany the feature. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if event_name == "pull_request": | ||
| commits = list(repo.iter_commits("HEAD", max_count=20)) | ||
| else: | ||
| commits = [head] |
There was a problem hiding this comment.
On pull_request events actions/checkout uses the auto-generated merge ref, so main() iterates commits starting from that merge (lines 161‑164) rather than the branch commits. GitHub’s synthetic merge messages have no component prefix or Signed-off-by, so every PR run will fail the prefix/signoff checks even when the actual PR commits are valid. Exclude the merge commit or check out the head SHA to avoid blocking all PRs with false failures.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
COMMIT_LINT.md (1)
41-52: Add language specifier to fenced code blocks.The example output code blocks lack a language specifier, which triggers markdownlint warnings (MD040). Consider adding
textorconsoleas the language identifier for consistency.**Success:** -``` +```text ✅ Commit prefix validation passed.Failure:
-+text
❌ Commit deadbeef12 failed:
Subject prefix 'wrong_prefix:' does not match files changed.
Expected one of: router:Commit prefix validation failed.
.github/workflows/commit-lint.yaml (1)
3-8: Consider whether theeditedtrigger is intentional.The
editedtrigger fires when the PR title or description is changed, not just when commits are updated. This may cause redundant workflow runs. If the intent is only to lint commits when they change,opened,synchronize, andreopenedare sufficient.on: pull_request: - types: [opened, synchronize, reopened, edited] + types: [opened, synchronize, reopened] push: branches: - master.github/scripts/commit_prefix_check.py (4)
20-20: GlobalRepoinstantiation may cause issues.Instantiating
Repo(".")at module level makes the script fail if imported from a different working directory or during unit testing. Consider moving this intomain()and passing the repo as a parameter to functions that need it, or wrap it in a lazy initialization.-repo = Repo(".") +def get_repo(): + return Repo(".")Then in
main():def main(): + repo = get_repo() head = repo.head.commit ...
34-38: PotentialIndexErroron malformed paths.If a path is exactly
"plugins/"or"plugins"without a subdirectory,parts[1]will raise anIndexError. While unlikely in practice, adding a guard improves robustness.if p.startswith("plugins/"): parts = p.split("/") + if len(parts) < 2 or not parts[1]: + continue prefix = parts[1] prefixes.add(f"{prefix}:") continue
66-68: Rename ambiguous variablelto improve readability.The variable name
lis flagged by Ruff (E741) as ambiguous because it can be confused with1orI. Use a more descriptive name likeline.# Normalize and discard empty lines - lines = [l.strip() for l in body.splitlines() if l.strip()] + lines = [line.strip() for line in body.splitlines() if line.strip()] - prefix_lines = [l for l in lines if PREFIX_RE.match(l)] + prefix_lines = [line for line in lines if PREFIX_RE.match(line)]
111-114: Remove unnecessarypassstatement.The
passstatement is redundant here since the block has a comment. While the comment clarifies intent, thepasscan be removed.# If due to multiple sign-offs, tests expect validate_commit() to still PASS # So we do NOT return False here. # validate_commit ignores multi signoff warnings. - pass
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/scripts/commit_prefix_check.py(1 hunks).github/workflows/commit-lint.yaml(1 hunks)COMMIT_LINT.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
COMMIT_LINT.md
[uncategorized] ~71-~71: The official name of this software platform is spelled with a capital “H”.
Context: ...ript is automatically run in CI via the .github/workflows/commit-lint.yaml workflow on...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
COMMIT_LINT.md
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.7)
.github/scripts/commit_prefix_check.py
1-1: Shebang is present but file is not executable
(EXE001)
66-66: Ambiguous variable name: l
(E741)
68-68: Ambiguous variable name: l
(E741)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
🔇 Additional comments (4)
COMMIT_LINT.md (1)
1-78: Documentation is comprehensive and well-structured.The documentation clearly explains prerequisites, local execution, behavior modes, exit codes, and CI integration. The reference to
CONTRIBUTING.mdfor commit format is appropriate..github/workflows/commit-lint.yaml (1)
14-30: Workflow structure is appropriate.The workflow correctly checks out with sufficient depth (50) for commit history analysis, sets up Python, installs the required dependency, and runs the linting script. The configuration is straightforward and functional.
.github/scripts/commit_prefix_check.py (2)
136-141: Strict validation fails commits touching multiple components.This always fails when files from multiple components are modified, even if the subject prefix matches one of them. This seems intentional per the comment, but may be overly strict for legitimate refactoring commits that touch multiple areas. Verify this matches the project's desired behavior.
157-178: Main function logic is correct.The function properly handles both CI (pull_request) and local/push modes, aggregates errors across commits, and provides clear output with appropriate exit codes.
…ive PR Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
|
Only thing I'd say here is we used old versions of actions which then had to be updated after merge - potentially they have issues so need to make sure we use latest. |
This PR adds a new commit message linter
Overview
The commit prefix checker (
commit_prefix_check.py) validates commit messages according to Fluent Bit standards:Prerequisites
Install the required Python dependency:
Running Locally
From the repository root directory, run:
Behavior
GITHUB_EVENT_NAMEenvironment variable is set topull_request, it validates the last 20 commits0if validation passes1if validation failsExample Output
Success:
Failure:
Testing Specific Commits
To test a specific commit, you can temporarily checkout that commit:
CI Integration
This script is automatically run in CI via the
.github/workflows/commit-lint.yamlworkflow on:masterbranchCommit Message Format
See CONTRIBUTING.md for the full commit message format requirements.
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.