-
-
Notifications
You must be signed in to change notification settings - Fork 88
[qa] Standardize commit messages using commitizen #110 #560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[qa] Standardize commit messages using commitizen #110 #560
Conversation
This change integrates Commitizen into openwisp_utils to standardize how commit messages are written across the project. It introduces an interactive commit workflow that guides contributors to use the correct OpenWISP commit format, ensures commit titles are properly structured, and enforces the presence of an issue preference. The commit message footer is generated automatically using the provided issue number, improving consistency and making commits easier to review and track. Fixes openwisp#110
WalkthroughAdds an OpenWisp Commitizen plugin, a top-level module export, a new Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer as Dev
participant CLI as "Commitizen CLI"
participant Plugin as "OpenWispCommitizen"
participant Repo as "SCM / Repo"
Dev->>CLI: run commitizen
CLI->>Plugin: invoke questions()
Plugin->>Dev: prompt type / custom_prefix? / title / body
Dev-->>Plugin: answers
Plugin->>Plugin: message(answers) -> assemble header, body, "Fixes #<issue>"
Plugin->>Plugin: validate_commit_message(message)
alt valid
Plugin->>CLI: return commit message
CLI->>Repo: create commit with message
Repo->>Repo: update changelog on bump (if configured)
else invalid
Plugin->>CLI: format_error_message -> show errors
CLI->>Dev: display validation errors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (14)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (4)
openwisp_utils/commitizen/openwisp.py (4)
4-4: Clarify the character range in the regex.The character class
[a-z0-9!:-]uses a range!:-which matches ASCII characters from!(33) to:(58), including various special characters like",#,$, etc. If the intent is to only allow specific punctuation, consider using an explicit list like[a-z0-9!:_-]for clarity.🔎 Suggested improvement for explicitness
-_CUSTOM_PREFIX_RE = re.compile(r"^[a-z0-9!:-]+$") +_CUSTOM_PREFIX_RE = re.compile(r"^[a-z0-9!:_-]+$")
16-26: Add ClassVar annotation for the class attribute.The static analysis correctly identifies that this mutable class attribute should be annotated with
typing.ClassVarto indicate it's a class-level constant.🔎 Proposed fix
+from typing import ClassVar + class OpenWispCommitizen(BaseCommitizen): """ Commitizen plugin for OpenWISP commit conventions. """ # Single source for allowed prefixes - ALLOWED_PREFIXES = [ + ALLOWED_PREFIXES: ClassVar[list[str]] = [ "feature", "change", "fix", "docs", "test", "ci", "chore", "qa", "other", ]
36-44: Simplify the string concatenation.Line 43 has an unusual string concatenation with a space between the two string literals. While syntactically correct, it's clearer to use a single string.
🔎 Proposed improvement
if not _TITLE_ISSUE_RE.search(value): - return "Please add the issue number at the end of the title " "(e.g. #104)." + return "Please add the issue number at the end of the title (e.g. #104)." return True
138-149: Prefix themessageparameter with an underscore.The
messageparameter is required by theBaseCommitizen.format_error_messageinterface but is currently unused. Since the method intentionally returns a static error message regardless of input, prefix the parameter with_messageto explicitly mark it as intentionally unused and comply with the interface contract.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.cz.tomlcz_openwisp.pyopenwisp_utils/commitizen/openwisp.pysetup.py
🧰 Additional context used
🧬 Code graph analysis (1)
cz_openwisp.py (1)
openwisp_utils/commitizen/openwisp.py (1)
OpenWispCommitizen(10-187)
🪛 Ruff (0.14.10)
openwisp_utils/commitizen/openwisp.py
16-26: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
138-138: Unused method argument: message
(ARG002)
🔇 Additional comments (5)
.cz.toml (1)
1-5: LGTM! Configuration is well-structured.The Commitizen configuration properly sets up the OpenWISP plugin with SCM version provider and changelog generation on version bumps.
cz_openwisp.py (1)
1-3: LGTM! Clean re-export module.The module correctly exposes the OpenWispCommitizen plugin for Commitizen's plugin system.
setup.py (2)
18-18: Module registration looks correct.The
py_modules=["cz_openwisp"]properly registers the standalone module for the plugin system.
25-27: Plugin entry point is correctly configured.The commitizen.plugin entry point properly wires the OpenWispCommitizen class to the "openwisp" plugin name.
openwisp_utils/commitizen/openwisp.py (1)
186-187: LGTM! Constructor correctly initializes the base class.
This commit fixes issues discovered after opening the PR, including updating the Commitizen example and info output, pinning the Commitizen dependency to a compatible version range, and correcting minor configuration problems. Fixes openwisp#110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
openwisp_utils/commitizen/openwisp.py (2)
95-131: Update docstring to match actual validation behavior.The docstring claims "Why and How sections must be present and non-empty," but the validation only checks that the body is non-empty (lines 117-119). It doesn't enforce any "Why:" or "How:" section structure. Update the docstring to accurately describe what the method validates.
🔎 Proposed fix
def validate_commit_message(self, message: str) -> bool: - """Enforce OpenWISP commit rules: - Title must start with a capital letter - Why and How sections must be present and non-empty""" + """ + Enforce OpenWISP commit rules: + - Header must match the format: [prefix] Capitalized title #<issue> + - Body must be present and non-empty + - Footer must contain "Fixes #<issue>" + - Issue number in title and footer must match + """
79-93: Add defensive null check for regex match.Line 91 directly accesses
match.group(1)without verifying thatmatchis notNone. While_validate_titleshould prevent this scenario, defensive programming requires a null check to handle edge cases where validation might be bypassed or fail.🔎 Proposed fix
def message(self, answers): if answers["change_type"] == "other": prefix_value = answers["custom_prefix"] else: prefix_value = answers["change_type"] prefix = f"[{prefix_value}]" title = answers["title"].strip() body = answers["how"].strip() # Extract issue number from title match = _TITLE_ISSUE_RE.search(title) + if not match: + raise ValueError( + "Title must contain an issue number (e.g., #123). " + "This should have been caught by validation." + ) issue_number = match.group(1) return f"{prefix} {title}\n\n" f"{body}\n\n" f"Fixes #{issue_number}"
🧹 Nitpick comments (2)
openwisp_utils/commitizen/openwisp.py (2)
15-25: Consider adding ClassVar annotation.The mutable class attribute
ALLOWED_PREFIXESshould be annotated withtyping.ClassVarto clarify it's a class-level constant shared across instances.🔎 Proposed refactor
+from typing import ClassVar + import re from commitizen.cz.base import BaseCommitizen _CUSTOM_PREFIX_RE = re.compile(r"^[a-z0-9!:-]+$") _TITLE_ISSUE_RE = re.compile(r"#(\d+)$") _HEADER_RE = re.compile(r"^\[[a-z0-9:!-]+\] [A-Z].*\s#\d+$") _FIXES_RE = re.compile(r"^Fixes #(\d+)$", re.MULTILINE) class OpenWispCommitizen(BaseCommitizen): """Commitizen plugin for OpenWISP commit conventions.""" # Single source for allowed prefixes - ALLOWED_PREFIXES = [ + ALLOWED_PREFIXES: ClassVar[list[str]] = [ "feature", "change", "fix", "docs", "test", "ci", "chore", "qa", "other", ]Based on static analysis.
133-144: Remove unused parameter.The
messageparameter is declared but never used in the method body. Consider removing it since the method returns a static error message.🔎 Proposed fix
- def format_error_message(self, message: str) -> str: + def format_error_message(self) -> str: return ( "Invalid commit message.\n\n" "Expected format:\n\n" "[prefix] Title starting with capital letter #<issue>\n\n" "<commit body>\n\n" "Fixes #<issue>\n\n" "Example:\n" "[feature] Add subnet import support #104\n\n" "Add support for importing multiple subnets from a CSV file.\n\n" "Fixes #104" )Based on static analysis.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
openwisp_utils/commitizen/openwisp.pysetup.py
🧰 Additional context used
🪛 Ruff (0.14.10)
openwisp_utils/commitizen/openwisp.py
15-25: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
133-133: Unused method argument: message
(ARG002)
⏰ 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). (14)
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
🔇 Additional comments (9)
setup.py (3)
18-18: LGTM!The
py_modulesentry correctly references the top-levelcz_openwisp.pymodule, enabling the commitizen plugin entry point to importOpenWispCommitizen.
24-27: LGTM!The commitizen plugin entry point is correctly configured. The mapping enables users to use the custom plugin via
cz commitwith thename = "openwisp"configuration in.cz.toml.
38-38: LGTM! Version constraint addresses past concern.The version constraint
>=4.0.0,<5.0.0correctly locks the dependency to the 4.x series, preventing potential breakage from future BaseCommitizen API changes. This addresses the concern raised in the previous review.openwisp_utils/commitizen/openwisp.py (6)
1-9: LGTM!The imports and regex patterns are well-defined for validating the OpenWISP commit convention. The patterns correctly enforce the bracketed prefix format, issue number extraction, and Fixes footer validation.
27-33: LGTM! Past issue resolved.The validation correctly enforces non-empty, lowercase custom prefixes matching the allowed pattern. The trailing spaces issue from the previous review has been fixed.
35-43: LGTM!The title validation correctly enforces the OpenWISP convention: non-empty, capitalized first letter, and mandatory issue reference at the end (e.g.,
#104).
45-77: LGTM!The interactive questions are well-structured with appropriate validation for each field. The conditional
custom_prefixprompt provides flexibility while maintaining structure. The implementation is now consistent with the documented format shown inexample()andinfo().
146-178: LGTM! Past issues resolved.The documentation methods are now internally consistent:
example()demonstrates the actual commit formatschema_pattern()correctly excludes "other" from the validation pattern (since it's only an interactive option)info()provides clear guidance on the conventionThese align well with the implementation in
questions()andmessage().
180-181: LGTM!The initialization correctly delegates to the parent
BaseCommitizenclass, following the standard Commitizen plugin pattern.
Fixed the error shown due to flake8 Fixes openwisp#110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
openwisp_utils/commitizen/openwisp.py (5)
15-25: Add ClassVar annotation for mutable class attribute.As flagged by static analysis, mutable class attributes should be annotated with
typing.ClassVarto clearly indicate they are class-level variables rather than instance attributes.🔎 Proposed fix
+from typing import ClassVar + from commitizen.cz.base import BaseCommitizen _CUSTOM_PREFIX_RE = re.compile(r"^[a-z0-9!:-]+$")"""Commitizen plugin for OpenWISP commit conventions.""" # Single source for allowed prefixes - ALLOWED_PREFIXES = [ + ALLOWED_PREFIXES: ClassVar[list[str]] = [ "feature", "change", "fix",
27-33: Improve error message clarity.The error message at line 32 only states "Prefix must be lowercase" but doesn't explain which characters are allowed. According to
_CUSTOM_PREFIX_RE, the prefix must be lowercase alphanumeric with optional punctuation (:,!,-).🔎 Proposed fix
if not _CUSTOM_PREFIX_RE.match(value): - return "Prefix must be lowercase." + return "Prefix must be lowercase and may only contain letters, digits, or punctuation (: ! -)." return True
35-43: Minor: Fix string formatting.Line 42 has awkward string concatenation with an unnecessary space before the parenthetical example.
🔎 Proposed fix
if not _TITLE_ISSUE_RE.search(value): - return "Please add the issue number at the end of the title " "(e.g. #104)." + return "Please add the issue number at the end of the title (e.g., #104)." return True
79-93: Consider defensive null check for regex match.While the validation in
_validate_titleensures an issue number exists, defensive programming suggests adding a null check for the regex match result. If validation is bypassed or the regex pattern changes, line 91 would raise anAttributeError.🔎 Proposed fix
# Extract issue number from title match = _TITLE_ISSUE_RE.search(title) + if not match: + raise ValueError("Title validation failed: missing issue number") issue_number = match.group(1)
132-143: Mark unused parameter with underscore prefix.As flagged by static analysis, the
messageparameter is unused. If this is required by theBaseCommitizeninterface, prefix it with an underscore to indicate it's intentionally unused.🔎 Proposed fix
- def format_error_message(self, message: str) -> str: + def format_error_message(self, _message: str) -> str: return ( "Invalid commit message.\n\n" "Expected format:\n\n"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
openwisp_utils/commitizen/openwisp.py
🧰 Additional context used
🪛 Ruff (0.14.10)
openwisp_utils/commitizen/openwisp.py
15-25: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
132-132: Unused method argument: message
(ARG002)
⏰ 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). (14)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
🔇 Additional comments (5)
openwisp_utils/commitizen/openwisp.py (5)
1-9: LGTM!The imports and regex constants are well-defined and appropriate for the validation requirements.
45-77: LGTM!The interactive question flow is well-structured with appropriate validations and conditional logic for custom prefixes.
95-130: LGTM!The validation logic is comprehensive and correctly enforces all required commit message constraints: proper header format, non-empty body, presence of Fixes footer, and consistent issue number references.
145-177: LGTM!The documentation methods are well-aligned with the implementation and provide clear guidance on the commit message format and allowed types.
179-180: LGTM!Standard initialization correctly delegates to the parent class.
While the issue number is already enforced earlier in the workflow, I’ve added a defensive check when extracting it from the commit title to avoid relying solely on upstream validation and prevent unexpected errors in future refactors. Fixes openwisp#110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openwisp_utils/commitizen/openwisp.py (1)
14-25: Add ClassVar annotation for mutable class attribute.
ALLOWED_PREFIXESis a mutable class attribute and should be annotated withtyping.ClassVarto clarify it's shared across all instances and follows Python typing best practices.🔎 Proposed fix
+from typing import ClassVar import re from commitizen.cz.base import BaseCommitizen _CUSTOM_PREFIX_RE = re.compile(r"^[a-z0-9!:-]+$") _TITLE_ISSUE_RE = re.compile(r"#(\d+)$") _HEADER_RE = re.compile(r"^\[[a-z0-9:!-]+\] [A-Z].*\s#\d+$") _FIXES_RE = re.compile(r"^Fixes #(\d+)$", re.MULTILINE) class OpenWispCommitizen(BaseCommitizen): """Commitizen plugin for OpenWISP commit conventions.""" # Single source for allowed prefixes - ALLOWED_PREFIXES = [ + ALLOWED_PREFIXES: ClassVar[list[str]] = [ "feature", "change", "fix", "docs", "test", "ci", "chore", "qa", "other", ]Based on static analysis hints.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
openwisp_utils/commitizen/openwisp.py
🧰 Additional context used
🪛 Ruff (0.14.10)
openwisp_utils/commitizen/openwisp.py
15-25: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
92-94: Avoid specifying long messages outside the exception class
(TRY003)
136-136: Unused method argument: message
(ARG002)
⏰ 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). (14)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
🔇 Additional comments (7)
openwisp_utils/commitizen/openwisp.py (7)
27-43: LGTM: Validation methods are well-implemented.Both
_validate_custom_prefixand_validate_titleprovide clear validation with helpful error messages. The logic correctly enforces the OpenWISP commit conventions.
45-77: LGTM: Interactive prompts align with documented format.The
questions()method provides a clear, guided workflow. The conditional custom prefix prompt (when "other" is selected) and validation callbacks ensure users create properly formatted commits.
79-97: LGTM: Message assembly includes proper null check.The defensive null check (lines 91-95) prevents potential AttributeError and provides a clear error message. The message assembly correctly formats the commit with prefix, title, body, and Fixes footer.
Note: Static analysis suggests extracting the exception message to a constant, but for a single use case this is an optional refactor.
99-134: LGTM: Comprehensive validation logic.The validation correctly enforces all OpenWISP commit conventions: proper header format, non-empty body, required Fixes footer, and consistent issue number references between title and footer. The logic flow is clear and thorough.
136-147: LGTM: Clear error formatting.The error message provides a clear explanation of the expected format with a concrete example.
Note: Static analysis flags the unused
messageparameter, but this is likely required by theBaseCommitizeninterface and is acceptable.
149-181: LGTM: Documentation methods provide clear guidance.The
example(),schema(),schema_pattern(), andinfo()methods consistently document the OpenWISP commit convention. Theschema_patterncorrectly excludes "other" as an actual commit type while allowing the predefined types.
183-184: LGTM: Proper initialization.
Ensure both interactive and final validation require a space before the issue number to avoid false positives. Fixes openwisp#110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
openwisp_utils/commitizen/openwisp.py (2)
15-25: Consider addingClassVarannotation for type safety.The
ALLOWED_PREFIXESclass attribute would benefit from atyping.ClassVarannotation to clearly indicate it's a class-level constant and improve type checking.🔎 Proposed refactor
+from typing import ClassVar + from commitizen.cz.base import BaseCommitizen _CUSTOM_PREFIX_RE = re.compile(r"^[a-z0-9!:-]+$")"""Commitizen plugin for OpenWISP commit conventions.""" # Single source for allowed prefixes - ALLOWED_PREFIXES = [ + ALLOWED_PREFIXES: ClassVar[list[str]] = [ "feature", "change",Based on static analysis hint RUF012.
79-97: Good defensive programming with the null check.The message assembly correctly handles all cases and includes a defensive check for missing issue numbers.
The inline exception message at lines 92-94 could be moved to a constant for consistency with style guides, though this is a minor style preference.
🔎 Optional refactor
+_TITLE_ISSUE_MISSING_ERROR = ( + "Commit title must end with an issue reference like #<issue_number>." +) + + class OpenWispCommitizen(BaseCommitizen):# Extract issue number from title match = _TITLE_ISSUE_RE.search(title) if not match: - raise ValueError( - "Commit title must end with an issue reference like #<issue_number>." - ) + raise ValueError(_TITLE_ISSUE_MISSING_ERROR) issue_number = match.group(1)Based on static analysis hint TRY003.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
openwisp_utils/commitizen/openwisp.py
🧰 Additional context used
🪛 Ruff (0.14.10)
openwisp_utils/commitizen/openwisp.py
15-25: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
92-94: Avoid specifying long messages outside the exception class
(TRY003)
136-136: Unused method argument: message
(ARG002)
⏰ 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). (14)
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
🔇 Additional comments (7)
openwisp_utils/commitizen/openwisp.py (7)
1-9: LGTM! Regex patterns are well-defined and consistent.The patterns correctly enforce OpenWISP commit conventions, and the previous inconsistency between
_TITLE_ISSUE_REand_HEADER_REhas been resolved—both now require a space before the issue number.
27-43: LGTM! Validation methods are well-implemented.Both validators correctly enforce the OpenWISP commit conventions with clear, helpful error messages.
45-77: LGTM! Interactive prompts are well-structured.The question flow correctly guides users through the commit creation process, with appropriate validation at each step and conditional custom prefix input.
99-134: LGTM! Comprehensive validation logic.The validation correctly enforces all OpenWISP commit conventions, including matching issue numbers between the title and footer, non-empty body, and proper header format.
149-181: LGTM! Helper methods provide clear guidance.The
example(),schema(),schema_pattern(), andinfo()methods provide consistent, comprehensive information about OpenWISP commit conventions. Theschema_pattern()correctly excludes "other" from valid commit types.
183-184: LGTM! Proper initialization.Correctly delegates to the parent class constructor.
136-147: Verify if themessageparameter is required by theBaseCommitizeninterface.Static analysis flags the
messageparameter as unused (ARG002). Since this method overridesBaseCommitizen.format_error_message, the parameter signature must match the parent class. If the base class interface requires it, the parameter can be safely ignored; if not, consider removing it.
nemesifier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely interesting! We're being flooded with PRs but I am doing my best to review most of them. I will come back to this asap, in the meantime see my comments below.
setup.py
Outdated
| "swapper~=1.4.0", | ||
| # allow wider range here to avoid interfering with other modules | ||
| "urllib3>=2.0.0,<3.0.0", | ||
| "commitizen>=4.0.0,<5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to the qa section.
.cz.toml
Outdated
| name = "openwisp" | ||
| version_provider = "scm" | ||
| tag_format = "v$version" | ||
| update_changelog_on_bump = true No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a blank line at the end of this file please.
commitizen has been moved to the qa optional dependencies. Fixes openwisp#110
Checklist
Reference to Existing Issue
Closes #110.
Description of Changes
This PR introduces a standardized commit message workflow for OpenWISP by integrating Commitizen into openwisp-utils. The goal is to make commit messages more consistent, easier to review, and simpler for contributors to write correctly. This change adds a guided, interactive commit workflow using the
cz commitcommand. Contributors are prompted step by step to generate a commit message that follows OpenWISP conventions.The workflow enforces:
The following commit prefixes are supported and encouraged:
[feature], [change], [fix], [docs], [test], [ci], [qa], [chore]
If none of the predefined prefixes apply, contributors can select other and provide a custom prefix . This keeps the workflow flexible while still enforcing a consistent format.