Bug 1954177: Update mao dependency for webhook v1 changes #180
Conversation
|
@elmiko: This pull request references Bugzilla bug 1954177, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 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 kubernetes/test-infra repository. |
openshift-ci-robot
added
bugzilla/severity-high
|
@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: sunzhaohua2. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
this is going to need these 2 patches to merge, then be re-vendored to include them: |
|
hitting a small snag with one of the unit tests, seems like a function has changed signature in a upstream dependency |
elmiko
force-pushed
the
mao-update-a132ec00
branch
from
2fba488 to
8faed19
Compare
|
trying out a fix for the unit tests, still needs the golang update to ci |
|
/retest |
|
hmm, seems the unit tests are quite unhappy |
also adds a small fix for the controller-runtime refactor of the signals module into a new location, and manager start function.
elmiko
force-pushed
the
mao-update-a132ec00
branch
from
8faed19 to
186298c
Compare
|
think i've fixed the unhappy test |
|
Thank you @elmiko for taking care of updating MAO! I'll let someone more familiar with CAPO to merge this one. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: EmilienM 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
adduarte
left a comment
adduarte
left a comment
There was a problem hiding this comment.
Left some comments nothing worth regenerating the patch. just observations
| "fmt" | ||
| "strconv" | ||
| "time" | ||
|
|
There was a problem hiding this comment.
Gratuitous change? I get it, but the change does not change the binary output. and makes the patch longer. Not important
There was a problem hiding this comment.
My VSCode made an incredibly similar change to something recently. In general VSCode doesn't do its own code analysis but uses standard go tools. I suspect @elmiko didn't make this change, but either ran go imports or some tool ran it on his behalf?
There was a problem hiding this comment.
yeah, i think my auto-linter did this when i saved the file
|
/lgtm |
|
@elmiko: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged: These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Once unlinked, request a bug refresh with Bugzilla bug 1954177 has not been moved to the MODIFIED state. 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/test-infra repository. |
6 participants
also adds a small fix for the controller-runtime refactor of the signals
module into a new location.