🎉 Migrate repo to Kubebuilder v2 #1249
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vincepri The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/XXL
vincepri
force-pushed
the
kbv2
branch
8 times, most recently
from
983daa8 to
21580cc
Compare
|
@detiber fixed all comments 🙂 |
vincepri
force-pushed
the
kbv2
branch
2 times, most recently
from
f828bf4 to
720aea4
Compare
Signed-off-by: Vince Prignano <[email protected]>
Signed-off-by: Vince Prignano <[email protected]>
Signed-off-by: Vince Prignano <[email protected]>
Signed-off-by: Vince Prignano <[email protected]>
| type: object | ||
| status: | ||
| description: MachineStatus defines the observed state of Machine | ||
| description: / [MachineStatus] MachineStatus defines the observed state |
There was a problem hiding this comment.
This generation looks a bit off.
There was a problem hiding this comment.
This has been shown before as well, we were missing marks on the resources, I added them back given that we'll probably reference these from the gitbook
| openAPIV3Schema: | ||
| description: / [MachineSet] MachineSet ensures that a specified number of machines | ||
| replicas are running at any given time. | ||
| description: / [MachineSet] MachineSet is the Schema for the machinesets API |
There was a problem hiding this comment.
Same here with the generation looking off
| _ = context.Background() | ||
| _ = r.Log.WithValues("machinedeployment", req.NamespacedName) | ||
|
|
||
| // your logic here |
There was a problem hiding this comment.
Still a todo to migrate the controller logic here?
There was a problem hiding this comment.
Yeah, it's gonna be a different PR
|
lgtm, @chuckha mind giving this a second set of eyes? |
chuckha
left a comment
chuckha
left a comment
There was a problem hiding this comment.
Nice approach, next PR moves the logic from old controllers to new ones. I was trying to avoid the bazel modifications but I'm not sure I'll get away with that.
Just some comments on the copyright. If you're not using the k/k copyright validator then ignore 'em
Signed-off-by: Vince Prignano <[email protected]>
Signed-off-by: Vince Prignano <[email protected]>
|
@vincepri squash your commits and lgtm or if you don't want to squash i'll lgtm |
|
The idea behind that change was that people are responsible for squashing their own commits instead of the merge bot. It was not that we would keep all commits that the dev made while developing because they are usually not particularly meaningful. The general practice is 1 commit for changes 1 for generated file updates, however, if you're happy with these commits no way I'd block on it |
Signed-off-by: Vince Prignano <[email protected]>
Signed-off-by: Vince Prignano <[email protected]>
Signed-off-by: Vince Prignano <[email protected]>
Signed-off-by: Vince Prignano <[email protected]>
|
/lgtm Thanks! |
k8s-ci-robot
added
the
lgtm
4 participants
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
Release note: