-
Notifications
You must be signed in to change notification settings - Fork 0
Fix release note row rendering to skip missing fields (type/assignee/developers/PRs) #251
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
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Comment |
Co-authored-by: miroslavpojer <109138042+miroslavpojer@users.noreply.github.com>
…sts and docs Co-authored-by: miroslavpojer <109138042+miroslavpojer@users.noreply.github.com>
Co-authored-by: miroslavpojer <109138042+miroslavpojer@users.noreply.github.com>
…ssion Co-authored-by: miroslavpojer <109138042+miroslavpojer@users.noreply.github.com>
miroslavpojer
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.
- pulled
- code review - comments created
- local test run is passing
tests/unit/release_notes_generator/model/test_issue_record_row_formatting.py
Outdated
Show resolved
Hide resolved
tests/unit/release_notes_generator/model/test_issue_record_row_formatting.py
Outdated
Show resolved
Hide resolved
…ests Co-authored-by: miroslavpojer <109138042+miroslavpojer@users.noreply.github.com>
tests/unit/release_notes_generator/model/test_issue_record_row_formatting.py
Show resolved
Hide resolved
tmikula-dev
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.
I have two point of my finding during the PR:
- You are using a lot of templates. Look if T strings in python 3.14 would be interesting topic for you.
- Please try to keep the clean code up and module patterns same.
…onfiguration files
tmikula-dev
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.
Please find the comments bellow. They are at this point only as suggestions what worked for me. If it is working for you, it is fine with me. But please react to my comments before merging.
.github/copilot-instructions.md
Outdated
| - Add new sections/updates chronologically below | ||
| - Use clear headings like "## Update [date/commit]" or "## Changes Added" | ||
| - Each update should reference the commit hash that made the changes | ||
| - Purpose: Maintain full history of PR evolution for reviewers and future reference. |
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.
I do find your instructions for copilot to vocal and long. Some information is repeating, since it is best to have the least chars. I would run one prompt to shorten the instructions, so no info is repeating, the bullet points are not a sentences, just points. Claude has done a great job with mine and I had great results (much better than the first try with vocal and sentences version)
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.
We made a call about it. I have tried to use your style.
Overview
Release notes rendered dangling text fragments when issue metadata was missing, producing output like:
This PR implements intelligent suppression of empty field fragments so missing metadata results in clean output:
Release Notes
Changes
Core Implementation
Added
format_row_with_suppression()inrelease_notes_generator/utils/record_utils.py:{type}:prefix when type is missing (no "N/A" substitution)assigned to {assignees}phrase when assignees list is emptydeveloped by {developers} in {pull-requests}phrase when both are emptyin {pull-requests}suffix when only PRs are missingconstants.pyfor maintainabilityUpdated
IssueRecord.to_chapter_row(): Changed type fallback from'N/A'to'', delegates formatting to suppression helperUpdated
HierarchyIssueRecord.to_chapter_row(): Changed type fallback from'None'to'', delegates formatting to suppression helperTesting & Documentation
test_issue_record_row_formatting.py, following project test style conventions and reusing existing fixtures fromconftest.pydocs/features/custom_row_formats.mddocumenting empty field suppression behavior with examples, clarifying that suppression only applies to specific placeholders ({type},{assignees},{developers},{pull-requests})Template Coupling Note: Suppression rules target default row format patterns. Custom templates with different phrase structures may not benefit from all suppression rules.
Original prompt
This section details on the original issue you should resolve
<issue_title>Fix release note row rendering to skip missing fields (type/assignee/developers/PRs)</issue_title>
<issue_description>### Feature Description
Update release-note row rendering so that when issue metadata is missing, the generator omits the entire related text fragment (prefix/label phrase), instead of printing placeholders as empty strings.
Concrete example:
Actual (today):
- N/A: AbsaOSS/generate-release-notes#231 _Dependency Dashboard_ author is @renovate[bot] assigned to developed by inExpected (after fix):
- AbsaOSS/generate-release-notes#231 _Dependency Dashboard_ author is @renovate[bot]This should apply to issue rows (and, if applicable, hierarchy issue rows) generated by the Action’s row-format templates.
Problem / Opportunity
The generated release notes can contain redundant, confusing, or broken text segments when certain GitHub fields are missing:
N/A:(noise; looks like a bug).assigned tofollowed by nothing (dangling phrase).developed by in(dangling phrase).This reduces readability and professionalism of release notes and forces manual cleanup for otherwise valid issues (e.g., Renovate bot dashboard issues).
Who benefits:
Acceptance Criteria
N/Aand must not start withN/A:whenissue.typeis absent.{type}:prefix (or equivalent prefix fragment) must be omitted entirely.assigned toat all.developed bynorinfragments (nodeveloped by in).Proposed Solution
High-level approach:
.format()into strings that include constant phrases around empty placeholders.{type}is empty/missing → omit the entire “type prefix” fragment (not substituteN/A).{assignees}is empty → omit the entire “assigned to …” fragment.{developers}or{pull-requests}is empty → omit the entire “developed by … in …” fragment.row-format-issue)row-format-hierarchy-issue) where the same placeholders/fragments exist.Expected code touchpoints (permalinks):
N/Aand always emits potentially-empty fields:IssueRecord.to_chapter_row()generate-release-notes/release_notes_generator/model/record/issue_record.py
Lines 120 to 154 in cfb1544
type = "None") and empty-string risks:HierarchyIssueRecord.to_chapter_row()generate-release-notes/release_notes_generator/model/record/hierarchy_issue_record.py
Lines 108 to 139 in cfb1544
action.ymlrow-format defaultsgenerate-release-notes/action.yml
Lines 91 to 109 in cfb1544
Expected docs touchpoints:
docs/features/custom_row_formats.mdgenerate-release-notes/docs/features/custom_row_formats.md
Lines 1 to 24 in cfb1544
Expected tests:
tests/unit/release_notes_generator/builder/test_release_notes_builder.pygenerate-release-notes/tests/unit/release_notes_generator/builder/test_release_notes_builder.py
Lines 1491 to 1545 in cfb1544
Dependencies / Related
No response
Additional Context
Relevant reference docs / constants:
generate-release-notes/docs/features/custom_row_formats.md
Lines 1 to 24 in cfb1544
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.