Skip to content

Update build root and run go fix#776

Open
AlexNPavel wants to merge 1 commit into
openshift:mainfrom
AlexNPavel:gofix
Open

Update build root and run go fix#776
AlexNPavel wants to merge 1 commit into
openshift:mainfrom
AlexNPavel:gofix

Conversation

@AlexNPavel

@AlexNPavel AlexNPavel commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Update build root image to golang 1.26 and run the go fix command on the project.

Summary by CodeRabbit

  • Refactor

    • Improved internal string-building and URL handling for better performance and clarity
    • Replaced some utility code with standard library calls for maintainability
    • Minor test simplifications for clearer assertions
  • Chores

    • Updated build environment to Go 1.26 and OpenShift 5.0

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 92b5c953-c5e1-429d-bfb9-dee08ea70c6e

📥 Commits

Reviewing files that changed from the base of the PR and between 8ab8c9a and 0f181ad.

📒 Files selected for processing (9)
  • .ci-operator.yaml
  • cmd/release-controller-api/http.go
  • pkg/cmd/release-mirror-cleanup-controller/controller.go
  • pkg/cmd/release-reimport-controller/controller.go
  • pkg/jira/jira.go
  • pkg/jira/jira_test.go
  • pkg/release-controller/machine_os_tags.go
  • pkg/release-controller/prowjob.go
  • pkg/release-controller/types.go

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting


📝 Walkthrough

Walkthrough

This PR modernizes utility patterns: replaces custom loops/concatenation with Go stdlib helpers (slices, strings.Builder, SplitSeq, CutPrefix, wg.Go), removes omitempty from a changelog JSON tag, and updates the CI build root image tag.

Changes

Go Stdlib and Efficiency Modernization

Layer / File(s) Summary
Build image version update
.ci-operator.yaml
build_root_image.tag updated to rhel-9-release-golang-1.26-openshift-5.0.
Release controller API concurrency & HTML rendering
cmd/release-controller-api/http.go
Changelog goroutines started via wg.Go(...); nodeImageStreams goroutine uses wg.Go; team approvals HTML now built with strings.Builder.
API JSON tag change
pkg/release-controller/types.go
APIReleaseInfo.ChangeLogJson JSON tag removed omitempty so changeLogJson is always emitted.
String parsing helpers
pkg/release-controller/machine_os_tags.go, pkg/release-controller/prowjob.go
Use strings.SplitSeq and strings.CutPrefix for annotation and URL prefix parsing.
Imagestream logging builder adoption
pkg/cmd/release-mirror-cleanup-controller/controller.go, pkg/cmd/release-reimport-controller/controller.go
Imagestream name lists are assembled with strings.Builder instead of repeated concatenation.
Jira helpers & tests
pkg/jira/jira.go, pkg/jira/jira_test.go
Use slices.Contains for label membership; simplify TestSimpleBackoff loop and clamp logic.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I nibble bugs and tidy strings,
wg.Go hops while the builder sings,
JSON keeps its little key,
CI's image leaps to twenty-six — whee! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Update build root and run go fix' directly matches the PR description and accurately summarizes both main objectives of the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from bradmwilliams and hoxhaeris June 9, 2026 00:15
@openshift-ci

openshift-ci Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlexNPavel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 9, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
cmd/release-controller-api/http.go (1)

709-766: sync.WaitGroup.Go is valid under Go 1.26 (introduced in Go 1.25)
wg.Go(...) on a sync.WaitGroup is not a Go 1.26-only addition—it was introduced in Go 1.25—so this should compile when building with Go 1.26. The remaining concern is just consistency: the same block uses both wg.Add(1)/defer wg.Done() (lines 696-707) and wg.Go(...) (lines 709-766).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/release-controller-api/http.go` around lines 709 - 766, The code mixes
two waitgroup patterns (wg.Add(1)/defer wg.Done() and wg.Go(...)); pick one for
consistency and apply it to both blocks: either convert the earlier block that
uses wg.Add(1)/defer wg.Done() to use wg.Go(func() { ... }) or change this
wg.Go(...) block to follow the wg.Add/Done pattern. Locate the uses of wg.Add,
wg.Done and wg.Go and make them consistent (prefer wg.Go where closures are used
to avoid forgetting Done), keeping the same closure body and preserving context
checks and the nodeImageErr/nodeImageStreams assignments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/release-controller/types.go`:
- Line 56: The ChangeLogJson field in the release struct (ChangeLogJson
ChangeLog) was changed to always serialize which causes an empty ChangeLog to be
returned when renderJSON.err != nil; revert this unintended API-breaking change
by restoring the omitempty tag on the ChangeLogJson field
(json:"changeLogJson,omitempty") in pkg/release-controller/types.go or
alternatively make ChangeLogJson a pointer and ensure the handler in
cmd/release-controller-api/http.go sets it to nil when renderJSON.err != nil so
the field is omitted — update the struct tag or the handler logic around
renderJSON.err accordingly.

---

Nitpick comments:
In `@cmd/release-controller-api/http.go`:
- Around line 709-766: The code mixes two waitgroup patterns (wg.Add(1)/defer
wg.Done() and wg.Go(...)); pick one for consistency and apply it to both blocks:
either convert the earlier block that uses wg.Add(1)/defer wg.Done() to use
wg.Go(func() { ... }) or change this wg.Go(...) block to follow the wg.Add/Done
pattern. Locate the uses of wg.Add, wg.Done and wg.Go and make them consistent
(prefer wg.Go where closures are used to avoid forgetting Done), keeping the
same closure body and preserving context checks and the
nodeImageErr/nodeImageStreams assignments.
🪄 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: Enterprise

Run ID: 481943a9-186a-4683-bff5-10f3bba69fe2

📥 Commits

Reviewing files that changed from the base of the PR and between 8ab8c9a and 98d3126.

📒 Files selected for processing (9)
  • .ci-operator.yaml
  • cmd/release-controller-api/http.go
  • pkg/cmd/release-mirror-cleanup-controller/controller.go
  • pkg/cmd/release-reimport-controller/controller.go
  • pkg/jira/jira.go
  • pkg/jira/jira_test.go
  • pkg/release-controller/machine_os_tags.go
  • pkg/release-controller/prowjob.go
  • pkg/release-controller/types.go

Comment thread pkg/release-controller/types.go
Update build root image to golang 1.26 and run the `go fix` command on
the project.
@AlexNPavel

Copy link
Copy Markdown
Contributor Author

@coderabbitai summary

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Summary regeneration triggered.

@openshift-ci

openshift-ci Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@AlexNPavel: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant