-
Notifications
You must be signed in to change notification settings - Fork 24
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.
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
Bind Proxmoxmachine Webhook #55
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
edit: (on second thought, no)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're aware that this is double pasted?
These code blocks are exactly the same.
cluster-api-provider-proxmox/cmd/main.go
Line 143 in 7b1af3e
if err = (&webhook.ProxmoxCluster{}).SetupWebhookWithManager(mgr); err != nil { |
cluster-api-provider-proxmox/cmd/main.go
Line 147 in 7b1af3e
if err = (&webhook.ProxmoxMachine{}).SetupWebhookWithManager(mgr); err != nil { |
What is that? |
yeah, that's webhook for ProxmoxCluster and the other is for ProxmoxMachine |
Lines 143 to 146 and Lines 147 to 150 are the exact same code block. To me it looks like you're just pasting the same block for no reason. |
not exactly:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be going blind.
Summary:
In This PR #42, the machine webhook is introduced/
but the binding was not implemented.