deprecating registry-replacer for the openshift org - #4913
Conversation
|
/retest-required |
WalkthroughAdds organization-level ignore support: introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)/bin/bash: line 1: golangci-lint: command not found Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/registry-replacer/main.go (1)
38-72: InitializeignoreOrgsbefore registering the flag.At line 178,
opts.ignoreOrgs.Strings()is called withoutignoreOrgsbeing initialized ingatherOptions(), causing a nil pointer panic at runtime.Proposed fix
o := &options{ ensureCorrectPromotionDockerfileIngoredRepos: &flagutil.Strings{}, ignoreRepos: &flagutil.Strings{}, + ignoreOrgs: &flagutil.Strings{}, }
|
/test checkconfig |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/registry-replacer/main.go (1)
53-71: InitializeignoreOrgsto prevent a startup panic.Line 71 registers
o.ignoreOrgswithflag.Var(), but the options literal never initializes it, causing a nil pointer panic. Initialize it alongside the otherflagutil.Stringsfields.Fix
o := &options{ ensureCorrectPromotionDockerfileIngoredRepos: &flagutil.Strings{}, ignoreRepos: &flagutil.Strings{}, + ignoreOrgs: &flagutil.Strings{}, }
|
/lgtm |
|
/override ci/prow/images |
|
Scheduling required tests: |
|
@Prucek: Overrode contexts on behalf of Prucek: ci/prow/e2e, ci/prow/images 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bear-redhat, droslean, jmguzik, Prucek 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 |
|
/override ci/prow/e2e |
|
@Prucek: Overrode contexts on behalf of Prucek: ci/prow/e2e 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. |
a1cf5a1
into
openshift:main
|
@Prucek: The following test failed, say
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. |
Follow up of: #4890, #4851