OCPBUGS-60989: adm release info: Print commit url if no associated PR number #2085
Conversation
openshift-ci-robot
added
jira/severity-moderate
|
@ardaguclu: This pull request references Jira Issue OCPBUGS-60989, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughUpdates mergeLogForRepo to conditionally parse pull-request numbers only when a PR match exists. Non-PR merge commits are now processed without setting PullRequest (left at zero). Invalid PR number parsing still returns an error. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant mergeLogForRepo
participant Commit
participant Logger
Caller->>mergeLogForRepo: Process merge logs
loop For each Commit
mergeLogForRepo->>Commit: Inspect merge message
alt PR pattern found (len(m) ≥ 2)
mergeLogForRepo->>mergeLogForRepo: Parse PR number
alt Parse fails
mergeLogForRepo-->>Caller: Error (invalid PR)
else Parse succeeds
mergeLogForRepo->>mergeLogForRepo: Set PullRequest = parsed ID
end
else No PR pattern
mergeLogForRepo->>Logger: Log "no pull-request"
mergeLogForRepo->>mergeLogForRepo: Leave PullRequest = 0
end
end
mergeLogForRepo-->>Caller: Completed list
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 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 (
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ardaguclu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/jira refresh |
openshift-ci-robot
added
the
jira/valid-bug
|
@ardaguclu: This pull request references Jira Issue OCPBUGS-60989, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
removed
the
jira/invalid-bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pkg/cli/admin/release/git.go (3)
307-308: Log message contradicts behavior; update wording and verbosityWe’re no longer omitting these commits. Suggest clarifying and lowering noise.
Apply this diff:
- klog.V(2).Infof("Omitted commit %s which has no pull-request", mergeCommit.Commit) + klog.V(4).Infof("Commit %s has no associated pull-request; leaving PullRequest=0", mergeCommit.Commit)
195-199: Document sentinel semantics for PullRequestMake it explicit that 0 means “no PR”. Helps downstream code and future readers.
Apply this diff:
type MergeCommit struct { CommitDate time.Time Commit string ParentCommits []string - PullRequest int + // PullRequest is the associated PR number when detected; 0 means no PR. + PullRequest int Refs RefList
259-261: Use klog verbosity API directly instead of manual gatingAvoid unconditional Info-level logging guarded by Enabled(). Use V(5).Infof.
Apply this diff:
- if klog.V(5).Enabled() { - klog.Infof("Got commit info:\n%s", strconv.Quote(out)) - } + klog.V(5).Infof("Got commit info:\n%s", strconv.Quote(out))
📜 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 (1)
pkg/cli/admin/release/git.go(1 hunks)
🔇 Additional comments (2)
pkg/cli/admin/release/git.go (2)
301-306: Good guard: PR parsing only when a match existsThis avoids panics and stops discarding non-PR commits. Leaving PullRequest at 0 is sensible.
301-306: No action required: commits without PRs are retained and display commit URLs for non-PR entries.
|
I'm closing this PR in favor of #2086 (which is better approach) |
|
@ardaguclu: Closed this PR. DetailsIn response to this:
Instructions 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. |
|
@ardaguclu: This pull request references Jira Issue OCPBUGS-60989. The bug has been updated to no longer refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
2 participants
oc adm release info --changelogdiscards the commits, if no associated pull request number is found. This is problematic because discarded commits makes it difficult to find the regression in the changelogs to investigate.So that this PR prints commit message (with commit url) to at least show the change in the output.
Summary by CodeRabbit