Feature/165 introduce regimes prepare default one #166
Conversation
- Implemented default regime.
- Implemented default regime.
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Workflow Run
participant Action as GH Action (composite)
participant Gen as ReleaseNotesGenerator
participant RF as DefaultRecordFactory
participant RB as DefaultReleaseNotesBuilder
User->>Action: inputs (including regime="default")
note right of Action:#lightblue Forward INPUT_REGIME to generator
Action->>Gen: generate()
Gen->>RF: RF.generate(github, minedData)
RF-->>Gen: records (issues / PRs / commits)
Gen->>RB: RB.build(records, changelogUrl, customChapters)
RB-->>Gen: release notes (string)
Gen-->>Action: notes
Action-->>User: output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
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 |
miroslavpojer
deleted the
feature/165-Introduce-regimes---prepare-default-one
branch
![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
🧹 Nitpick comments (13)
action.yml (1)
35-38: Clarify and future-proof the new input’s description.If only "default" is supported today, explicitly say “Options: default (currently the only option)” to avoid implying others exist already. Consider validating allowed values in code (see ActionInputs.get_regime()).
Apply:
- description: 'Regime of the release notes generation. Options: default.' + description: 'Regime of the release notes generation. Options: default (currently the only option).'release_notes_generator/record/record_factory.py (1)
33-44: Nit: unifying terminology in docstrings.Use consistent “GitHub” spelling in parameter docs (“Github” vs “GitHub”).
README.md (1)
84-98: Regime docs are clear; consider adding usage snippet.Add with: regime: default to the YAML examples for discoverability.
release_notes_generator/action_inputs.py (2)
165-171: Use the class constant for the default; avoid duplicating literals.Apply:
- return get_action_input("regime", "default") # type: ignore[return-value] # default defined + return get_action_input("regime", ActionInputs.REGIME_DEFAULT) # type: ignore[return-value]
377-379: Validate allowed values, not just type.Since only “default” is supported, reject or warn on unknown values to avoid silent misconfiguration.
Apply:
- regime = ActionInputs.get_regime() - ActionInputs.validate_input(regime, str, "Regime must be a string.", errors) + regime = ActionInputs.get_regime() + ActionInputs.validate_input(regime, str, "Regime must be a string.", errors) + if isinstance(regime, str) and regime.strip().lower() not in {ActionInputs.REGIME_DEFAULT}: + errors.append(f"Unsupported regime '{regime}'. Supported: {ActionInputs.REGIME_DEFAULT}.")release_notes_generator/builder/base_builder.py (1)
17-19: Nit: docstring grammar.“responsible for building of the release notes” → “responsible for building the release notes”.
Apply:
-This module contains the ReleaseNotesBuilder class which is responsible for building of the release notes. +This module contains the ReleaseNotesBuilder class which is responsible for building the release notes.tests/release_notes/test_release_notes_builder.py (1)
324-346: DRY: consider a small helper/fixture to construct the builder.Many tests repeat the same 3-arg constructor. A fixture or helper will reduce noise and ease future signature changes.
Also applies to: 360-365, 370-375, 548-554, 568-575, 586-593, 604-611, 623-629, 641-647, 657-663, 673-679, 691-697, 710-717, 729-736, 749-756, 767-773, 785-792, 803-810, 823-829, 843-850, 861-868, 880-886, 898-904, 915-921, 933-939
tests/release_notes/test_record_factory.py (1)
31-32: Path and API updates look correct; one tiny style nit.
- Import/patch path migrations to default_record_factory and instance-based usage of DefaultRecordFactory().generate(...) are consistent across tests. LGTM.
- Nit: add a space after the comma in the call on Line 232 for readability.
Apply:
- records = DefaultRecordFactory().generate(mock_github_client,data) + records = DefaultRecordFactory().generate(mock_github_client, data)Also applies to: 174-176, 192-192, 212-214, 232-233, 281-283, 316-318, 346-347, 376-379, 397-397, 427-427
release_notes_generator/generator.py (1)
95-101: Regime selection scaffold is fine; consider a mapping for easy extension.Using a direct instantiation now is OK. For future regimes, prefer a dict-based registry keyed by ActionInputs.get_regime() over commented match scaffolding to keep this method small and testable. I can sketch this when you’re ready.
release_notes_generator/record/default_record_factory.py (4)
125-150: Optional: speed up commit registration on large datasets._current approach scans all records per commit. Consider pre-indexing merge_commit_sha -> target record(s) once to reduce O(R*C) scans, especially when records scale.
49-57: Minor doc nit.Param doc says “GitHub” while type is Github; also mention that keys may be int (issue/PR) or str (commit sha). Non-blocking.
59-59: Ruff ARG001 will clear with the fallback usage above.If you prefer not to create a fallback PR record, drop the skip_rec parameter instead to satisfy Ruff.
37-40: Security note (out of scope but related): get_issues_for_pr disables TLS verification.Since DefaultRecordFactory relies on get_issues_for_pr, consider removing verify=False in pull_request_utils and trusting system CAs. Keeps the surface safer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(3 hunks)action.yml(2 hunks)release_notes_generator/action_inputs.py(4 hunks)release_notes_generator/builder/base_builder.py(1 hunks)release_notes_generator/builder/default_builder.py(1 hunks)release_notes_generator/generator.py(2 hunks)release_notes_generator/record/default_record_factory.py(1 hunks)release_notes_generator/record/record_factory.py(1 hunks)tests/release_notes/test_record_factory.py(12 hunks)tests/release_notes/test_release_notes_builder.py(27 hunks)tests/test_release_notes_generator.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
release_notes_generator/record/default_record_factory.py (8)
release_notes_generator/model/commit_record.py (2)
CommitRecord(13-71)commit(45-50)release_notes_generator/model/issue_record.py (3)
issue(63-68)get_pull_request_numbers(164-170)get_pull_request(137-147)release_notes_generator/model/mined_data.py (1)
MinedData(38-56)release_notes_generator/action_inputs.py (2)
ActionInputs(61-469)get_skip_release_notes_labels(202-210)release_notes_generator/model/pull_request_record.py (1)
is_commit_sha_present(173-181)release_notes_generator/utils/decorators.py (1)
safe_call_decorator(51-82)release_notes_generator/utils/github_rate_limiter.py (1)
GithubRateLimiter(30-76)release_notes_generator/utils/pull_request_utils.py (2)
get_issues_for_pr(52-72)extract_issue_numbers_from_body(32-48)
release_notes_generator/action_inputs.py (1)
release_notes_generator/utils/gh_action.py (1)
get_action_input(26-38)
tests/release_notes/test_release_notes_builder.py (1)
release_notes_generator/builder/default_builder.py (1)
DefaultReleaseNotesBuilder(30-71)
release_notes_generator/builder/base_builder.py (5)
release_notes_generator/action_inputs.py (3)
ActionInputs(61-469)get_warnings(267-272)get_print_empty_chapters(275-280)release_notes_generator/generator.py (1)
custom_chapters(61-63)release_notes_generator/model/custom_chapters.py (1)
CustomChapters(32-85)release_notes_generator/builder/default_builder.py (1)
build(37-71)release_notes_generator/builder.py (1)
ReleaseNotesBuilder(32-86)
release_notes_generator/builder/default_builder.py (1)
release_notes_generator/builder/base_builder.py (1)
ReleaseNotesBuilder(27-53)
release_notes_generator/record/record_factory.py (4)
release_notes_generator/model/mined_data.py (1)
MinedData(38-56)release_notes_generator/model/record.py (1)
Record(29-183)release_notes_generator/generator.py (1)
generate(70-106)release_notes_generator/record/default_record_factory.py (1)
generate(49-123)
release_notes_generator/generator.py (4)
release_notes_generator/builder/base_builder.py (2)
ReleaseNotesBuilder(27-53)build(48-53)release_notes_generator/builder/default_builder.py (2)
DefaultReleaseNotesBuilder(30-71)build(37-71)release_notes_generator/record/default_record_factory.py (2)
DefaultRecordFactory(44-165)generate(49-123)release_notes_generator/record/record_factory.py (2)
RecordFactory(28-44)generate(34-44)
tests/release_notes/test_record_factory.py (3)
release_notes_generator/record/default_record_factory.py (1)
DefaultRecordFactory(44-165)release_notes_generator/generator.py (1)
generate(70-106)release_notes_generator/record/record_factory.py (1)
generate(34-44)
🪛 Ruff (0.12.2)
release_notes_generator/record/default_record_factory.py
59-59: Unused function argument: skip_rec
(ARG001)
🔇 Additional comments (11)
action.yml (1)
140-140: Env passthrough looks correct.INPUT_REGIME aligns with get_action_input("regime", ...).
release_notes_generator/record/record_factory.py (1)
28-35: Good abstraction: interface-only base with abstract generate().This cleanly separates the contract from implementations.
tests/test_release_notes_generator.py (1)
70-71: Approve patch target path updatesNo stale references to the old record_factory.get_issues_for_pr remain in the codebase.
release_notes_generator/action_inputs.py (3)
66-67: Good to centralize the default as a constant.
436-436: Helpful debug log.Logging the selected regime is useful for troubleshooting.
165-171: Ignore default handling check
Theget_action_inputimplementation correctly applies the provided default viaos.getenv(key, default=…), so there is no formatting bug and defaults will be returned as expected.Likely an incorrect or invalid review comment.
release_notes_generator/builder/base_builder.py (1)
44-46: Good centralization of flags.Reading warnings/print_empty_chapters in the base keeps subclasses lean.
release_notes_generator/builder/default_builder.py (1)
30-71: Subclassing the base builder is clean; logic unchanged.Build flow remains intact and benefits from the new base.
tests/release_notes/test_release_notes_builder.py (2)
18-18: Imports updated to DefaultReleaseNotesBuilder — good.
337-338: Approve: no remaining references to the old ActionInputs path
Verified via ripgrep thatrelease_notes_generator.builder.ActionInputsis no longer present anywhere in the codebase.release_notes_generator/generator.py (1)
108-116: Builder injection LGTM._clean handoff via _get_rls_notes_builder and explicit typing improves testability and isolates future builder variants.
| | Name | Description | Required | Default | | ||
| |------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------------------------------------------| | ||
| | `GITHUB_TOKEN` | Your GitHub token for authentication. Store it as a secret and reference it in the workflow file as secrets.GITHUB_TOKEN. | Yes | | | ||
| | `tag-name` | The name of the tag for which you want to generate release notes. This should be the same as the tag name used in the release workflow. | Yes | | | ||
| | `from-tag-name` | The name of the tag from which you want to generate release notes. | No | '' | | ||
| | `chapters` | An YAML array defining chapters and corresponding labels for categorization. Each chapter should have a title and a label matching your GitHub issues and PRs. | Yes | | | ||
| | `regime` | Controls the regime of the action. Options: `default`. See more about the [Regimes](#regimes). | No | `default` | | ||
| | `row-format-issue` | The format of the row for the issue in the release notes. The format can contain placeholders for the issue `number`, `title`, and issues `pull-requests`. The placeholders are case-sensitive. | No | `"{number} _{title}_ in {pull-requests}"` | | ||
| | `row-format-pr` | The format of the row for the PR in the release notes. The format can contain placeholders for the PR `number`, and `title`. The placeholders are case-sensitive. | No | `"{number} _{title}_"` | | ||
| | `row-format-link-pr` | If defined `true`, the PR row will begin with a `"PR: "` string. Otherwise, no prefix will be added. | No | true | | ||
| | `duplicity-scope` | Set to `custom` to allow duplicity issue lines to be shown only in custom chapters. Options: `custom`, `service`, `both`, `none`. | No | `both` | | ||
| | `duplicity-icon` | The icon used to indicate duplicity issue lines in the release notes. Icon will be placed at the beginning of the line. | No | `🔔` | | ||
| | `published-at` | Set to true to enable the use of the `published-at` timestamp as the reference point for searching closed issues and PRs, instead of the `created-at` date of the latest release. If first release, repository creation date is used. | No | false | | ||
| | `skip-release-notes-labels` | List labels used for detection if issues or pull requests are ignored in the Release Notes generation process. Example: `skip-release-notes, question`. | No | `skip-release-notes` | | ||
| | `verbose` | Set to true to enable verbose logging for detailed output during the action's execution. | No | false | | ||
| | `release-notes-title` | The title of the release notes section in the PR description. | No | `[Rr]elease [Nn]otes:` | | ||
| | `coderabbit-support-active` | Enable CodeRabbit support. If true, the action will use CodeRabbit to generate release notes. | No | false | | ||
| | `coderabbit-release-notes-title` | The title of the CodeRabbit summary in the PR body. Value supports regex. | No | `Summary by CodeRabbit` | | ||
| | `coderabbit-summary-ignore-groups` | List of "group names" to be ignored by release notes detection logic. Example: `Documentation, Tests, Chores, Bug Fixes`. | No | '' | | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Docs/action mismatch: chapters “Required” flag conflicts with action.yml and runtime validation.
- README marks chapters as Required: Yes.
- action.yml sets required: false with default ''.
- validate_inputs() errors when chapters are empty, effectively making them required.
This will confuse users and can fail runs unexpectedly. Align both sources (prefer marking chapters required in action.yml, or relax validation).
To confirm all references, search for where chapters are validated and how the action input is declared:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'get_chapters\(|Chapters must be a valid yaml array' && rg -nP -C2 '^ chapters:\n(\s+.*\n){1,6}\s+required:\s+\w+' action.ymlLength of output: 2907
Require chapters input consistently
chapters is marked required in README but defined as optional (required: false, default '') in action.yml and then enforced as required at runtime in release_notes_generator/action_inputs.py. Update action.yml to required: true (or adjust validation/docs) to eliminate this mismatch.
🤖 Prompt for AI Agents
README.md around lines 45 to 64: the README lists `chapters` as required but
action.yml currently marks it optional and runtime code enforces it required,
causing a mismatch; fix by updating action.yml to set `chapters` required: true
(and remove or adjust the empty default) so metadata matches docs and runtime,
or alternatively update README and runtime validation to mark `chapters`
optional and provide a clear default — ensure the chosen approach is applied
consistently in action.yml, README.md, and
release_notes_generator/action_inputs.py.
| def register_pull_request(pull: PullRequest, skip_rec: bool) -> None: | ||
| detected_issues = extract_issue_numbers_from_body(pull) | ||
| logger.debug("Detected issues - from body: %s", detected_issues) | ||
| detected_issues.update(safe_call(get_issues_for_pr)(pull_number=pull.number)) | ||
| logger.debug("Detected issues - final: %s", detected_issues) | ||
|
|
There was a problem hiding this comment.
Handle GraphQL failures safely and avoid dropping PRs when linked issues are missing.
- If safe_call(get_issues_for_pr)(...) returns None, set.update(None) will raise TypeError and the condition in Line 104 misclassifies the PR. Coalesce to [].
- When referenced issues aren’t present/fetchable, the code logs “stand-alone PR ...” but never creates a PullRequestRecord, so the PR disappears from output.
The patch below:
- Guards against None from safe_call.
- Tracks whether the PR was registered to any issue and falls back to creating a PullRequestRecord once if not.
- Uses skip_rec, resolving the Ruff ARG001 warning.
Apply:
- def register_pull_request(pull: PullRequest, skip_rec: bool) -> None:
- detected_issues = extract_issue_numbers_from_body(pull)
- logger.debug("Detected issues - from body: %s", detected_issues)
- detected_issues.update(safe_call(get_issues_for_pr)(pull_number=pull.number))
- logger.debug("Detected issues - final: %s", detected_issues)
+ def register_pull_request(pull: PullRequest, skip_rec: bool) -> None:
+ detected_issues = extract_issue_numbers_from_body(pull)
+ logger.debug("Detected issues - from body: %s", detected_issues)
+ issues_from_api = safe_call(get_issues_for_pr)(pull_number=pull.number) or []
+ detected_issues.update(issues_from_api)
+ logger.debug("Detected issues - final: %s", detected_issues)
+
+ registered_to_issue = False
@@
- if parent_issue_number in records:
+ if parent_issue_number in records:
cast(IssueRecord, records[parent_issue_number]).register_pull_request(pull)
logger.debug("Registering PR %d: %s to Issue %d", pull.number, pull.title, parent_issue_number)
+ registered_to_issue = True
else:
logger.debug(
"Registering stand-alone PR %d: %s as mentioned Issue %d not found.",
pull.number,
pull.title,
parent_issue_number,
)
+ # Fallback: if none of the mentioned issues could be registered/fetched, keep the PR as standalone
+ if not registered_to_issue and pull.number not in records:
+ records[pull.number] = PullRequestRecord(pull, skip=skip_rec)
+ logger.debug("Created fallback record for PR %d: %s", pull.number, pull.title)
@@
- for pull in data.pull_requests:
+ for pull in data.pull_requests:
pull_labels = [label.name for label in pull.get_labels()]
skip_record: bool = any(item in pull_labels for item in ActionInputs.get_skip_release_notes_labels())
-
- if not safe_call(get_issues_for_pr)(pull_number=pull.number) and not extract_issue_numbers_from_body(pull):
+ linked_from_api = safe_call(get_issues_for_pr)(pull_number=pull.number) or []
+ linked_from_body = extract_issue_numbers_from_body(pull)
+ if not linked_from_api and not linked_from_body:
records[pull.number] = PullRequestRecord(pull, skip=skip_record)
logger.debug("Created record for PR %d: %s", pull.number, pull.title)
else:
logger.debug("Registering pull number: %s, title : %s", pull.number, pull.title)
register_pull_request(pull, skip_record)Also applies to: 80-90, 99-110, 111-123
🧰 Tools
🪛 Ruff (0.12.2)
59-59: Unused function argument: skip_rec
(ARG001)
1 participant
Release Notes:
Summary by CodeRabbit
New Features
Documentation
Refactor
Fixes #165