Skip to content

bump to go1.25 and cleanup various debts #584

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.

Sign up for GitHub

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

Jump to bottom
Merged
k8s-ci-robot merged 1 commit into from
Jan 8, 2026
Merged

bump to go1.25 and cleanup various debts #584

k8s-ci-robot merged 1 commit into from
Jan 8, 2026

Conversation

@upodroid
Copy link
Member

@upodroid upodroid commented Jan 5, 2026 •
edited
Loading

This PR is busy, but it does the following:

  • bumps go to 1.25.5
  • bumps golangci-lint to v2 and addresses trivial findings, various Quick fixes are currently turned off and all ST checks are turned off as we never ran styletype checks before
  • Our go.mod was very huge because our tool deps appeared in the main go.mod. I now created a new tools go.mod and moved our prowimagebuilder which pulls in many dependencies to tools folder

Potential cleanup findings: #585

@netlify
Copy link

netlify bot commented Jan 5, 2026 •
edited
Loading

✅ Deploy Preview for k8s-prow ready!

Name Link
🔨 Latest commit 8113d2a
🔍 Latest deploy log https://app.netlify.com/projects/k8s-prow/deploys/695ec9e79171f1000843b3eb
😎 Deploy Preview https://deploy-preview-584--k8s-prow.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot k8s-ci-robot added area/deck Issues or PRs related to prow's deck component area/gerrit Issues or PRs related to prow's gerrit component labels Jan 5, 2026
@k8s-ci-robot k8s-ci-robot added the area/hook Issues or PRs related to prow's hook component label Jan 5, 2026
@k8s-ci-robot k8s-ci-robot added area/moonraker Issues or PRs related to prow's moonraker component area/peribolos Issues or PRs related to prow's peribolos component area/plugins Issues or PRs related to prow's plugins for the hook component area/pod-utilities Issues or PRs related to prow's pod-utilities component area/pubsub Issues or PRs related to prow's pubsub reporter component area/spyglass Issues or PRs related to prow's spyglass UI area/tide Issues or PRs related to prow's tide component size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 5, 2026
@stmcginnis
Copy link
Contributor

stmcginnis commented Jan 5, 2026

Hmm...

stat /home/prow/go/src/github.com/kubernetes-sigs/prow/hack/prowimagebuilder: directory not found

@upodroid
Copy link
Member Author

upodroid commented Jan 5, 2026

yeah, I forgot to amend the go flags for the integration tests

@stmcginnis
Copy link
Contributor

stmcginnis commented Jan 5, 2026

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 5, 2026
petr-muller
petr-muller reviewed Jan 5, 2026
View reviewed changes
pkg/config/prow-config-documented.yaml
@@ -1,255 +1,157 @@
branch-protection:
# AllowDeletions allows deletion of the protected branch by anyone with write access to the repository.
Copy link
Contributor

@petr-muller petr-muller Jan 5, 2026

Choose a reason for hiding this comment

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

We saw this in one of the other bump PRs, the annotated config was useful and it'd be great to find a way to keep it that way

Copy link
Member Author

@upodroid upodroid Jan 5, 2026

Choose a reason for hiding this comment

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

I'm not sure what's causing that to be dropped. I'll investigate it

Copy link
Contributor

@prad9192 prad9192 Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Hey noticed this from a tag, I did the changes here to preserve the comments - 3ab449d

If folks are good with the change ,we could either merge this PR - #580
or close it and make appropriate changes in this PR

@stmcginnis
Copy link
Contributor

stmcginnis commented Jan 5, 2026

Just to note, there is some overlap with #580

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 6, 2026
@upodroid
Copy link
Member Author

upodroid commented Jan 6, 2026

I didn't see that PR, happy to have it merged and then I can rebase mine

@stmcginnis
Copy link
Contributor

stmcginnis commented Jan 7, 2026

#580 has now merged. I think there are some things that we still need in this PR, once the merge conflicts are resolved.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2026
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2026
@stmcginnis
Copy link
Contributor

stmcginnis commented Jan 7, 2026

Looks great to me!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 7, 2026
matthyx
matthyx approved these changes Jan 7, 2026
View reviewed changes
Copy link
Contributor

@matthyx matthyx left a comment

Choose a reason for hiding this comment

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

LGTM, small nit, holding for @petr-muller
/hold

pkg/pubsub/subscriber/subscriber.go Outdated
// PubSub message.
var allowedApiClient *config.AllowedApiClient = nil
var requireTenantID bool = false
var requireTenantID = false
Copy link
Contributor

@matthyx matthyx Jan 7, 2026

Choose a reason for hiding this comment

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

or

Suggested change
var requireTenantID = false
var requireTenantID bool

Copy link
Member Author

@upodroid upodroid Jan 7, 2026

Choose a reason for hiding this comment

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

Fixed

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 7, 2026
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 7, 2026
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 7, 2026
@matthyx
Copy link
Contributor

matthyx commented Jan 7, 2026

/approve

petr-muller
petr-muller approved these changes Jan 8, 2026
View reviewed changes
Copy link
Contributor

@petr-muller petr-muller left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 8, 2026
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jan 8, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matthyx, petr-muller, upodroid

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:
  • OWNERS [matthyx,petr-muller]

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

@matthyx
Copy link
Contributor

matthyx commented Jan 8, 2026

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2026
@k8s-ci-robot k8s-ci-robot merged commit 1a02de8 into kubernetes-sigs:main Jan 8, 2026
11 checks passed
@kubevirt-bot kubevirt-bot mentioned this pull request Jan 14, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petr-muller petr-muller petr-muller approved these changes

@AlexNPavel AlexNPavel Awaiting requested review from AlexNPavel

@chases2 chases2 Awaiting requested review from chases2

@MadhavJivrajani MadhavJivrajani Awaiting requested review from MadhavJivrajani

@Priyankasaggu11929 Priyankasaggu11929 Awaiting requested review from Priyankasaggu11929

+2 more reviewers

@prad9192 prad9192 prad9192 left review comments

@matthyx matthyx matthyx approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@petr-muller petr-muller

@stmcginnis stmcginnis

@matthyx matthyx

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/deck Issues or PRs related to prow's deck component area/gerrit Issues or PRs related to prow's gerrit component area/hook Issues or PRs related to prow's hook component area/moonraker Issues or PRs related to prow's moonraker component area/peribolos Issues or PRs related to prow's peribolos component area/plugins Issues or PRs related to prow's plugins for the hook component area/pod-utilities Issues or PRs related to prow's pod-utilities component area/pubsub Issues or PRs related to prow's pubsub reporter component area/spyglass Issues or PRs related to prow's spyglass UI area/tide Issues or PRs related to prow's tide component cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@upodroid @stmcginnis @matthyx @k8s-ci-robot @petr-muller @prad9192