Skip to content

ci: enforce conventional commits for messages and pull request titles - #607

Open
leonardocarreras wants to merge 3 commits into
sogno-platform:masterfrom
leonardocarreras:ci/conventional-commits
Open

ci: enforce conventional commits for messages and pull request titles#607
leonardocarreras wants to merge 3 commits into
sogno-platform:masterfrom
leonardocarreras:ci/conventional-commits

Conversation

@leonardocarreras

Copy link
Copy Markdown
Contributor

Adds a commitlint config, a commit-msg hook and a CI check covering both commit messages and the pull request title. The title matters because it becomes the merge commit subject on master.

Today 217 of the 326 commits since v1.2.1 do not parse, and the newest commit on master bumps as a patch rather than a minor because its type is capitalised. This is a prerequisite for release-please, which derives the version and the changelog from these messages.

Depends on #509.

Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>

@github-actions github-actions Bot 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.

DPsim LLM review

Claim vs. code: matches the description.

TL;DR: One real issue remains: the new workflow file is missing the required SPDX header; the only other item is a documentation mismatch about what pre-commit install enables, while the compliance/scheduling checks otherwise surfaced no deeper correctness problems. The SPDX finding is the must-fix item for repository policy compliance.

Found 1 high, 1 medium (2 anchored to lines below).

🔴 Critical & high

  • Add SPDX license header to new workflow file [high · 95% confidence] in .github/workflows/compliance.yaml:1 (details inline)

🟡 Suggestions

  • Clarify pre-commit activation in contribution quick-start [medium · 82% confidence] in docs/hugo/content/en/docs/Contribution guidelines/index.md:18 (details inline)
Claim vs. implementation
  • Claimed: Enforce conventional commits for commit messages and pull request titles via commitlint, hooks, and CI.
  • Done: Adds commitlint config plus pre-commit/commit-msg and CI checks for commit messages, PR titles, no merge commits, and notebook output stripping, along with contributor docs.
  • Difference: none
How this review was produced

13 specialized finder passes raised 19 findings over the diff and the full changed sources. After de-duplication, 19 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 15 refuted as unsupported, 4 kept (0 tentative).

Refuted by verification:

  • Prefer consistent numbering style in ordered lists (CONTRIBUTING.md): All ordered-list items use the same Markdown style 1. throughout lines 7-24.
  • Clarify conventional commits requirement for PR titles (CONTRIBUTING.md): The file already states the PR title becomes the merge commit subject on master in the Conventional commits bullet.
  • Prefer consistent numbering style in ordered lists (docs/hugo/content/en/docs/Contribution guidelines/index.md): All ordered-list items in the file use the same Markdown style 1. on lines 17, 18, 19, 20, and 27.
  • Add missing newline at end of file (.pre-commit-config.yaml): The file ends with the final hook stanza and no missing trailing-newline defect is visible in the source text.
  • Fix inconsistent list numbering in Quick start section (CONTRIBUTING.md): The Quick start list is consistently numbered with 1. on every item, including lines 7-24.
  • Fix inconsistent list numbering in Quick start section (docs/hugo/content/en/docs/Contribution guidelines/index.md): The Quick start list is consistently numbered with 1. for each item, which is valid Markdown ordered-list syntax.
  • Fix typo in error message (.github/workflows/compliance.yaml): The file already contains the exact message at line 103, and it is a generic plural instruction for multiple offending commits.
  • Fix typo in error message (.pre-commit-config.yaml): The quoted message is present exactly as shown, including the colon, so there is no typo to fix in this file.
  • Replace hard-coded 'master' with 'main' for branch reference (.github/workflows/compliance.yaml): The workflow intentionally references upstream/master in the rebase instructions, and the trigger also accepts both master and main on line 5.
  • Add missing 'linear history' requirement to CONTRIBUTING.md (CONTRIBUTING.md): The document explicitly includes a Linear history requirement: merge commits in a feature branch are blocked by CI; rebase instead.
  • Add SPDX header to commitlint config file (.commitlintrc.yaml): The file already contains SPDX-FileCopyrightText and SPDX-License-Identifier headers on lines 1-2.
  • Add SPDX header to pre-commit config file (.pre-commit-config.yaml): This is a YAML config file with no SPDX convention applicable in-file, and the repository rule cited is not evidenced here.
  • Add SPDX header to CONTRIBUTING.md (CONTRIBUTING.md): This is a Markdown document, and the SPDX requirement is already mentioned in the Key requirements section at line 32.
  • Add SPDX license header to new pre-commit config file (.pre-commit-config.yaml): The file is not a newly added source requiring an SPDX header under the documented code-file convention.
  • Missing SPDX header in pre-commit configuration (.pre-commit-config.yaml): There is no SPDX header requirement shown for this YAML pre-commit config, and the file content itself is otherwise valid YAML.

Automated, non-blocking review. May be wrong. Models: find mistral-small-4-119b-2603, gpt-oss-120b → verify gpt-5.4-mini → final gpt-5.5.

@@ -0,0 +1,110 @@
name: Compliance checks

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.

Add SPDX license header to new workflow file
severity: high · confidence: 95%

Project guidelines require every new file to contain an SPDX license header. The newly added workflow file lacks the required SPDX-FileCopyrightText and SPDX-License-Identifier lines, which violates the licensing documentation policy.

Suggested fix: Insert an SPDX header at the top of the file, matching the format used in other new files.

Suggested change
name: Compliance checks
# SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, EONERC, RWTH Aachen University
# SPDX-License-Identifier: MPL-2.0

Checked against the source: new workflow file has no YAML-comment SPDX copyright or license lines anywhere

stage: process-compliance

# Quick start

1. Fork the repository and clone your fork.
1. Run `pre-commit install` to activate the automated checks (formatting, linear history guard, etc.).

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.

Clarify pre-commit activation in contribution quick-start
severity: medium · confidence: 82%

The quick-start section in docs/hugo/content/en/docs/Contribution guidelines/index.md instructs users to 'Run pre-commit install to activate the automated checks', but it does not specify that this installs both pre-commit and commit-msg hooks. The updated pre-commit-config.yaml now installs commit-msg hooks by default, which is essential for conventional commit enforcement. The documentation should reflect this to avoid confusion.

Suggested fix: Update the quick-start line to read: 'Run pre-commit install to activate the automated checks (formatting, commit message style, linear history guard, notebook output stripping).' to match the CONTRIBUTING.md phrasing and include all enforced checks.

Checked against the source: quick-start says only pre-commit install activates checks and never mentions the commit-msg hook

stage: tests-docs-coverage

@sonarqubecloud

Copy link
Copy Markdown

if [ -z "$NOTEBOOKS" ]; then
exit 0
fi
pip install nbconvert
if [ -z "$NOTEBOOKS" ]; then
exit 0
fi
pip install nbconvert
node-version: '20'

- name: Install commitlint
run: npm install --no-save @commitlint/cli @commitlint/config-conventional
node-version: '20'

- name: Install commitlint
run: npm install --no-save @commitlint/cli @commitlint/config-conventional
env:
PR_TITLE: ${{ github.event.pull_request.title }}
run: |
if ! printf '%s\n' "$PR_TITLE" | npx commitlint --verbose; then
env:
PR_TITLE: ${{ github.event.pull_request.title }}
run: |
if ! printf '%s\n' "$PR_TITLE" | npx commitlint --verbose; then
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.85%. Comparing base (47b236e) to head (222271d).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #607      +/-   ##
==========================================
+ Coverage   72.75%   72.85%   +0.09%     
==========================================
  Files         497      497              
  Lines       32305    32305              
  Branches    17470    17469       -1     
==========================================
+ Hits        23503    23535      +32     
+ Misses       8801     8689     -112     
- Partials        1       81      +80     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants