#128 - Allow to skip patch version in input assuming 0 #158
Conversation
- tag and from-tag inputs can be now defined as short versions. The `.0` is added automatically.
WalkthroughAdds a regex-based version tag normalizer and integrates it into action input parsing (TAG_NAME and FROM_TAG_NAME), updates README comments, and expands tests to cover full, shortened, empty, and invalid tag formats. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant GA as GitHub Actions Inputs
participant AI as ActionInputs
participant U as utils.normalize_version_tag
User->>GA: Provide TAG_NAME / FROM_TAG_NAME (e.g., "v1.2")
GA->>AI: get_tag_name() / get_from_tag_name()
AI->>U: normalize_version_tag(tag)
alt Valid short or full semver
U-->>AI: "vMAJOR.MINOR.PATCH" (e.g., "v1.2.0")
AI-->>GA: return normalized tag
else Empty input
U-->>AI: ""
AI-->>GA: return ""
else Invalid format
U-->>AI: ValueError
AI-->>GA: propagate error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(none) Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
release_notes_generator/utils/utils.py (1)
74-101: Don’t force-add a 'v' prefix; preserve user’s prefix and any suffixes.Unconditionally returning f"v{...}" breaks repos that tag without a 'v'. Preserve the input’s prefix and carry through prerelease/build parts. Consider renaming to a public helper while here.
-def _normalize_version_tag(tag: str) -> str: +def normalize_version_tag(tag: str) -> str: @@ - m = _SEMVER_SHORT_RE.match(tag) + m = _SEMVER_SHORT_RE.match(tag) if not m: raise ValueError( f"Invalid version tag format: {tag!r}. " "Expected vMAJOR.MINOR[.PATCH], e.g. 'v0.2' or 'v0.2.0'." ) - major = int(m.group("major")) - minor = int(m.group("minor")) - patch = int(m.group("patch")) if m.group("patch") is not None else 0 + prefix = m.group("prefix") or "" + major = int(m.group("major")) + minor = int(m.group("minor")) + patch = int(m.group("patch")) if m.group("patch") is not None else 0 + prerelease = m.group("prerelease") + build = m.group("build") + pre = f"-{prerelease}" if prerelease else "" + bld = f"+{build}" if build else "" - return f"v{major}.{minor}.{patch}" + return f"{prefix}{major}.{minor}.{patch}{pre}{bld}"Follow-up (tests): add cases for "V1.2", "v1.2.3-rc.1", "1.2.3+build.5", and "1.2-rc.1" → "1.2.0-rc.1".
release_notes_generator/action_inputs.py (1)
352-359: Catch invalid tag formats during validation instead of letting exceptions bubble.Aligns with existing pattern of aggregating validation errors.
- tag_name = ActionInputs.get_tag_name() - if not isinstance(tag_name, str) or not tag_name.strip(): - errors.append("Tag name must be a non-empty string.") - - from_tag_name = ActionInputs.get_from_tag_name() - if not isinstance(from_tag_name, str): - errors.append("From tag name must be a string.") + try: + tag_name = ActionInputs.get_tag_name() + except ValueError as e: + errors.append(str(e)) + tag_name = "" + if not isinstance(tag_name, str) or not tag_name.strip(): + errors.append("Tag name must be a non-empty string.") + + try: + from_tag_name = ActionInputs.get_from_tag_name() + except ValueError as e: + errors.append(str(e)) + from_tag_name = "" + if not isinstance(from_tag_name, str): + errors.append("From tag name must be a string.")
🧹 Nitpick comments (5)
README.md (1)
105-106: Clarify accepted short forms to match implementation/tests.Docs mention only "v0.2"/"v0.1"; tests and code accept bare "0.2"/"0.1" too. Update the inline comments to avoid confusion.
- tag-name: "v0.2.0" # accepts also v0.2 format when patch version is 0 - from-tag-name: "v0.1.0" # accepts also v0.1 format when patch version is 0 + tag-name: "v0.2.0" # also accepts v0.2 or 0.2 when patch version is 0 + from-tag-name: "v0.1.0" # also accepts v0.1 or 0.1 when patch version is 0release_notes_generator/action_inputs.py (2)
123-125: Use default to ensure str and drop type ignore.Also switch to the public helper name.
- raw = get_action_input(TAG_NAME) or "" - return _normalize_version_tag(raw) + raw = get_action_input(TAG_NAME, default="") + return normalize_version_tag(raw)
131-133: Drop the type ignore; default enforces str.Also use the public helper.
- raw = get_action_input(FROM_TAG_NAME, default="") - return _normalize_version_tag(raw) # type: ignore[arg-type] + raw = get_action_input(FROM_TAG_NAME, default="") + return normalize_version_tag(raw)tests/test_action_inputs.py (2)
116-124: Rename “shorted” → “shortened”.Minor grammar fix in test names.
-def test_get_tag_name_version_shorted_with_v(mocker): +def test_get_tag_name_version_shortened_with_v(mocker): @@ -def test_get_tag_name_version_shorted_no_v(mocker): +def test_get_tag_name_version_shortened_no_v(mocker):
143-151: Rename “shorted” → “shortened”.Keep naming consistent with the tag-name tests.
-def test_get_from_tag_name_version_shorted_with_v(mocker): +def test_get_from_tag_name_version_shortened_with_v(mocker): @@ -def test_get_from_tag_name_version_shorted_no_v(mocker): +def test_get_from_tag_name_version_shortened_no_v(mocker):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
README.md(1 hunks)release_notes_generator/action_inputs.py(3 hunks)release_notes_generator/utils/utils.py(2 hunks)tests/test_action_inputs.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_action_inputs.py (1)
release_notes_generator/action_inputs.py (2)
get_tag_name(119-124)get_from_tag_name(127-132)
release_notes_generator/action_inputs.py (2)
release_notes_generator/utils/utils.py (1)
_normalize_version_tag(74-100)release_notes_generator/utils/gh_action.py (1)
get_action_input(26-38)
🔇 Additional comments (4)
release_notes_generator/action_inputs.py (1)
420-423: LGTM: extra debug context.Logging from-tag improves diagnosability.
tests/test_action_inputs.py (3)
111-114: LGTM: full version passthrough covered.
126-135: LGTM: invalid format raises ValueError with clear message.
158-163: LGTM: mirrors invalid-format coverage for from-tag.
| _SEMVER_SHORT_RE = re.compile( | ||
| r""" | ||
| ^\s* # optional leading whitespace | ||
| v? # optional leading 'v' | ||
| (?P<major>\d+) # major | ||
| \. # dot | ||
| (?P<minor>\d+) # minor | ||
| (?:\.(?P<patch>\d+))? # optional .patch | ||
| \s*$ # optional trailing whitespace | ||
| """, | ||
| re.VERBOSE, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid enforcing lowercase 'v'; support uppercase 'V', pre-release, and build metadata.
Current regex disallows common tags like V1.2.3, v1.2.3-rc.1, or v1.2.3+build. It also makes it hard to preserve/omit the prefix consistently.
-_SEMVER_SHORT_RE = re.compile(
- r"""
- ^\s* # optional leading whitespace
- v? # optional leading 'v'
- (?P<major>\d+) # major
- \. # dot
- (?P<minor>\d+) # minor
- (?:\.(?P<patch>\d+))? # optional .patch
- \s*$ # optional trailing whitespace
-""",
- re.VERBOSE,
-)
+_SEMVER_SHORT_RE = re.compile(
+ r"""
+ ^\s*
+ (?P<prefix>[vV])? # optional leading 'v' or 'V'
+ (?P<major>0|[1-9]\d*) # major (no leading zeros unless zero)
+ \.
+ (?P<minor>0|[1-9]\d*) # minor
+ (?:\.(?P<patch>0|[1-9]\d*))? # optional .patch
+ (?:-(?P<prerelease>[0-9A-Za-z.-]+))? # optional -prerelease
+ (?:\+(?P<build>[0-9A-Za-z.-]+))? # optional +build
+ \s*$
+""",
+ re.VERBOSE,
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _SEMVER_SHORT_RE = re.compile( | |
| r""" | |
| ^\s* # optional leading whitespace | |
| v? # optional leading 'v' | |
| (?P<major>\d+) # major | |
| \. # dot | |
| (?P<minor>\d+) # minor | |
| (?:\.(?P<patch>\d+))? # optional .patch | |
| \s*$ # optional trailing whitespace | |
| """, | |
| re.VERBOSE, | |
| ) | |
| _SEMVER_SHORT_RE = re.compile( | |
| r""" | |
| ^\s* | |
| (?P<prefix>[vV])? # optional leading 'v' or 'V' | |
| (?P<major>0|[1-9]\d*) # major (no leading zeros unless zero) | |
| \. | |
| (?P<minor>0|[1-9]\d*) # minor | |
| (?:\.(?P<patch>0|[1-9]\d*))? # optional .patch | |
| (?:-(?P<prerelease>[0-9A-Za-z.-]+))? # optional -prerelease | |
| (?:\+(?P<build>[0-9A-Za-z.-]+))? # optional +build | |
| \s*$ | |
| """, | |
| re.VERBOSE, | |
| ) |
🤖 Prompt for AI Agents
In release_notes_generator/utils/utils.py around lines 60 to 71, the SEMVER
short regex currently only accepts a lowercase 'v' and no pre-release or build
metadata; update the pattern to accept an optional prefix (v or V) and optional
pre-release (-...) and build (+...) components and capture them with named
groups so the caller can consistently preserve or omit the prefix and metadata.
Concretely: change the leading v? to accept either case (use re.IGNORECASE or
[vV] and/or add a named group like (?P<prefix>v)?), add optional named groups
(?P<prerelease>-(?:[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))? and
(?P<build>\+(?:[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))? after the patch, keep patch
optional as before, and retain re.VERBOSE (or combine with re.IGNORECASE) so the
function can parse tags like V1.2.3, v1.2.3-rc.1, and v1.2.3+build and return
consistent captures for prefix, prerelease, and build.
There was a problem hiding this comment.
I want to like it is now.
There was a problem hiding this comment.
@miroslavpojer, understood! I respect your decision to keep the regex pattern simple and focused on your specific use case. The current implementation effectively handles the core requirement of allowing patch versions to be omitted while defaulting to .0, which aligns perfectly with the PR objectives.
Keeping it straightforward and avoiding additional complexity like pre-release versions or build metadata makes sense for your current needs.
🐰
nodding thoughtfully
✏️ Learnings added
Learnt from: miroslavpojer
PR: AbsaOSS/generate-release-notes#158
File: release_notes_generator/utils/utils.py:60-71
Timestamp: 2025-08-28T08:39:30.135Z
Learning: In the generate-release-notes project, the user miroslavpojer prefers to keep the SEMVER regex pattern simple, focusing only on basic major.minor[.patch] format with lowercase 'v' prefix support, rather than expanding it to handle uppercase 'V', pre-release versions, or build metadata.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
release_notes_generator/utils/utils.py (2)
87-94: Trim once and make the error message reflect accepted inputs.Single trim avoids double work, and error text should mention both with/without 'v' forms since you accept them.
- if not tag or tag.strip() == "": - return "" - - m = _SEMVER_SHORT_RE.match(tag) + s = tag.strip() + if not s: + return "" + + m = _SEMVER_SHORT_RE.match(s) if not m: - raise ValueError( - f"Invalid version tag format: {tag!r}. " "Expected vMAJOR.MINOR[.PATCH], e.g. 'v0.2' or 'v0.2.0'." - ) + raise ValueError( + f"Invalid version tag format: {tag!r}. " + "Expected [v]MAJOR.MINOR[.PATCH], e.g. 'v0.2', '0.2', '1.2.3', or 'v1.2.3'." + )
35-42: Docstring return description is incorrect for get_change_url.Function returns an empty string on missing repository, not None. Adjust wording for accuracy.
- @return: An optional string containing the URL to view the changes. Returns a None if the repository is not set. + @return: A string containing the URL to view the changes. Returns an empty string if the repository is not set.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
release_notes_generator/action_inputs.py(3 hunks)release_notes_generator/utils/utils.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- release_notes_generator/action_inputs.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: miroslavpojer
PR: AbsaOSS/generate-release-notes#158
File: release_notes_generator/utils/utils.py:60-71
Timestamp: 2025-08-28T08:39:30.135Z
Learning: In the generate-release-notes project, the user miroslavpojer prefers to keep the SEMVER regex pattern simple, focusing only on basic major.minor[.patch] format with lowercase 'v' prefix support, rather than expanding it to handle uppercase 'V', pre-release versions, or build metadata.
📚 Learning: 2025-08-28T08:39:30.135Z
Learnt from: miroslavpojer
PR: AbsaOSS/generate-release-notes#158
File: release_notes_generator/utils/utils.py:60-71
Timestamp: 2025-08-28T08:39:30.135Z
Learning: In the generate-release-notes project, the user miroslavpojer prefers to keep the SEMVER regex pattern simple, focusing only on basic major.minor[.patch] format with lowercase 'v' prefix support, rather than expanding it to handle uppercase 'V', pre-release versions, or build metadata.
Applied to files:
release_notes_generator/utils/utils.py
🔇 Additional comments (2)
release_notes_generator/utils/utils.py (2)
22-22: Import for regex usage — LGTM.
60-71: Regex scope matches the PR objective and stated preference.Pattern cleanly supports MAJOR.MINOR[.PATCH] with optional lowercase 'v' and rejects pre-release/build metadata, aligning with your preference to keep the regex simple and lowercase-only.
miroslavpojer
deleted the
feature/128-Allow-to-skip-patch-version-in-input-assuming-0
branch
Release Notes:
.0is added automatically.Fixes #128
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests