jira: include rejected payloads when verifying payload component bugs#751
jira: include rejected payloads when verifying payload component bugs#751rioliu-rh wants to merge 4 commits intoopenshift:mainfrom
Conversation
Previously, bugs were only moved to VERIFIED when the fix was included in an accepted release payload. This change also considers rejected payloads, so bugs can be verified as soon as blocking tests complete, regardless of whether they pass or fail. Bugs fixed in payload components are pre-merge verified by QA on ephemeral clusters via qe-approved/verified PR labels. The payload phase check is only used to confirm blocking tests have executed and the fix is present — not as an additional QA gate.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughsyncJira now considers both ReleasePhaseAccepted and ReleasePhaseRejected tags when selecting release tags for Jira verification. The verifier's default PR/ticket comment text was changed from "Fix included in accepted release " to "Fix included in release ", and comment deduplication now recognizes a legacy phrasing. Tests updated accordingly. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/release-controller/jira.go (1)
220-223: Consider renamingacceptedTagsto reflect mixed phases.Now that the slice includes accepted + rejected tags, the local name is a bit misleading.
♻️ Small clarity diff
- acceptedTags := releasecontroller.SortedRawReleaseTags(release, releasecontroller.ReleasePhaseAccepted, releasecontroller.ReleasePhaseRejected) - tag, prevTag := getNonVerifiedTagsJira(acceptedTags) + phaseFilteredTags := releasecontroller.SortedRawReleaseTags(release, releasecontroller.ReleasePhaseAccepted, releasecontroller.ReleasePhaseRejected) + tag, prevTag := getNonVerifiedTagsJira(phaseFilteredTags)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/release-controller/jira.go` around lines 220 - 223, The local variable acceptedTags is misleading because it actually contains both accepted and rejected phases; rename it (for example to acceptedAndRejectedTags or mixedPhaseTags) wherever it is declared and used (the call to releasecontroller.SortedRawReleaseTags and the subsequent getNonVerifiedTagsJira invocation and any log messages) to make intent clear and update the log message that references it (e.g., in klog.V(6).Infof) so names remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/release-controller/jira.go`:
- Around line 220-223: The local variable acceptedTags is misleading because it
actually contains both accepted and rejected phases; rename it (for example to
acceptedAndRejectedTags or mixedPhaseTags) wherever it is declared and used (the
call to releasecontroller.SortedRawReleaseTags and the subsequent
getNonVerifiedTagsJira invocation and any log messages) to make intent clear and
update the log message that references it (e.g., in klog.V(6).Infof) so names
remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ef6272c-15a0-492a-a6ab-12dd49690d6b
📒 Files selected for processing (1)
cmd/release-controller/jira.go
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
The message 'Fix included in accepted release X' was inaccurate when the fix is included in a rejected payload. Changed to 'Fix included in release X' which is correct for both accepted and rejected payloads.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/jira/jira.go`:
- Line 110: The new message variable (message := fmt.Sprintf("Fix included in
release %s", tagName)) uses updated wording but commentOnPR's dedupe only
matches the new literal, so existing comments using the legacy "accepted
release" wording can cause duplicate bot comments; update the dedupe logic in
commentOnPR (or the dedupe helper it calls) to consider both the new and legacy
message variants (e.g., by checking a regex or normalizing content) or normalize
existing comments before comparison so that messages containing either "Fix
included in release %s" or the prior "accepted release" phrasing with tagName
are treated as identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f0939932-f4e1-4a12-a19e-ed44411fb962
📒 Files selected for processing (2)
pkg/jira/jira.gopkg/jira/jira_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/jira/jira_test.go
After renaming 'Fix included in accepted release' to 'Fix included in release', the commentOnPR dedupe check would no longer match existing comments using the old wording, causing duplicate bot comments on reruns. Check both the new and legacy message variants to avoid duplicates.
cmd/release-controller/jira.go
Outdated
| // confirms that all blocking tests have been executed (accepted or rejected) and that the code | ||
| // fix is present in the payload, allowing the bug status to be automatically transitioned to | ||
| // VERIFIED and notifying assignees which payload contains their fix. | ||
| acceptedTags := releasecontroller.SortedRawReleaseTags(release, releasecontroller.ReleasePhaseAccepted, releasecontroller.ReleasePhaseRejected) |
There was a problem hiding this comment.
This is no longer just acceptedTags
There was a problem hiding this comment.
Renamed to completedTags to better reflect that both accepted and rejected phase tags are included.
| legacyMessage := strings.Replace(message, "in release ", "in accepted release ", 1) | ||
| for _, comment := range comments { | ||
| if strings.Contains(comment.Body, message) { | ||
| if strings.Contains(comment.Body, message) || strings.Contains(comment.Body, legacyMessage) { |
There was a problem hiding this comment.
There should be a test for this.
There was a problem hiding this comment.
Added TestCommentOnPRLegacyDedupe to verify that commentOnPR does not post a duplicate comment when an existing PR comment uses the legacy "Fix included in accepted release" wording.
- Rename acceptedTags to completedTags to better reflect that the slice contains both accepted and rejected phase tags (i.e. tags that have completed their blocking test cycle) - Add TestCommentOnPRLegacyDedupe to verify that commentOnPR does not post duplicate comments when an existing comment uses the legacy 'Fix included in accepted release' wording
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rioliu-rh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@rioliu-rh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
Previously, payload component bugs were only moved to VERIFIED when the fix landed in an accepted release payload. This change also considers rejected payloads, so bugs can be verified as soon as blocking tests complete — regardless of whether they pass or fail.
qe-approved/verifiedPR labelsAcceptedandRejectedpayload phases satisfy this condition: all blocking tests have run and the fix is confirmed in the payloadJira: OCPERT-358
Summary by CodeRabbit
Bug Fixes
Tests