Skip to content
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

Update kubebuilder to commit b6a0bf1b6040 #4215

Merged
merged 2 commits into from Nov 13, 2020
Merged

Update kubebuilder to commit b6a0bf1b6040 #4215

merged 2 commits into from Nov 13, 2020

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Nov 10, 2020 •
edited

Description of the change:

Motivation for the change:

  • Solve tech-debt and keep both projects aligned

@jmrodri
Copy link
Member

jmrodri commented Nov 10, 2020

I retitled this PR it used to be called Update kb 9c02d557f01b-to-b6a0bf1b6040 I changed it to Update kubebuilder to commit b6a0bf1b6040

@jmrodri jmrodri changed the title Update kb 9c02d557f01b-to-b6a0bf1b6040 Update kubebuilder to commit b6a0bf1b6040 Nov 10, 2020
jmrodri
jmrodri approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2020
@jberkhahn
Copy link
Contributor

would be nice to wait until 1711 is merged, but oh well :/

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Nov 11, 2020 •
edited

Hi @johananl,

would be nice to wait until 1711 is merged, but oh well :/

I cannot see any no reason for we wait for any specific new fix or change in the Kubebuilder side.
See that the idea is we keep both aligned. If we do not keeping doing these updates then, when we do that we will need to perform so many changes to bump that whcih will make this process be very very very complex to get done. It ensures quality and its maintainability. Note that we need to check all changes made there to see if it has nothing that should be addressed in the Ansible/Helm plugins as well.

@johananl
Copy link
Contributor

@camilamacedo86 wrong tag ;-) Tagging @jberkhahn.

@camilamacedo86 camilamacedo86 force-pushed the update-kb-latest branch 4 times, most recently from 1372821 to 8812579 Compare November 13, 2020 19:13
estroz
estroz suggested changes Nov 13, 2020
View reviewed changes
Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

Two nits, overall LGTM

changelog/fragments/update-kb-from-9c02d557f01b-to-c158f4fa4207.yaml Outdated
Comment on lines 5 to 6
(go/v2) Fix `create api` when only the controller is created.
More info: https://github.com/kubernetes-sigs/kubebuilder/pull/1770
Copy link
Member

Choose a reason for hiding this comment

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

The "More info: [link]" string is too verbose. I say we stick to the ([kubebuilder#X](https://github.com/kubernetes-sigs/kubebuilder/pull/)) format, ex.

Suggested change
(go/v2) Fix `create api` when only the controller is created.
More info: https://github.com/kubernetes-sigs/kubebuilder/pull/1770
(go/v2) Fixed controller imports scaffolded by `create api` when `--resource=false`
([kubebuilder#1770](https://github.com/kubernetes-sigs/kubebuilder/pull/1770))

internal/plugins/golang/v2/plugin.go Outdated
Comment on lines 39 to 42
_ plugin.Init = Plugin{}
_ plugin.CreateAPI = Plugin{}
_ plugin.CreateWebhook = Plugin{}
_ plugin.Edit = Plugin{}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_ plugin.Init = Plugin{}
_ plugin.CreateAPI = Plugin{}
_ plugin.CreateWebhook = Plugin{}
_ plugin.Edit = Plugin{}
_ plugin.Full = Plugin{}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good cather 👍

estroz
estroz approved these changes Nov 13, 2020
View reviewed changes
Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2020
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2020
@camilamacedo86 camilamacedo86 merged commit eb694f2 into operator-framework:master Nov 13, 2020
@camilamacedo86 camilamacedo86 deleted the update-kb-latest branch November 13, 2020 22:24
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 5, 2021
@camilamacedo86
Update kubebuilder to commit b6a0bf1b6040 (operator-framework#4215)
7f1b3ea 
**Description of the change:**
- Update kubebuilder dependency (See: kubernetes-sigs/kubebuilder@9c02d55...c158f4f)
- Align SDK with Kubebuilder and address bug fixes

**Motivation for the change:**
- Solve tech-debt and keep both projects aligned

Signed-off-by: reinvantveer <rein.van.t.veer@geodan.nl>
rearl-scwx pushed a commit to rearl-scwx/operator-sdk that referenced this pull request Feb 5, 2021
@camilamacedo86 @rearl-scwx
Update kubebuilder to commit b6a0bf1b6040 (operator-framework#4215)
85b4dec 
**Description of the change:**
- Update kubebuilder dependency (See: kubernetes-sigs/kubebuilder@9c02d55...c158f4f)
- Align SDK with Kubebuilder and address bug fixes

**Motivation for the change:**
- Solve tech-debt and keep both projects aligned

Signed-off-by: rearl <rearl@secureworks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmrodri jmrodri jmrodri approved these changes

@estroz estroz estroz approved these changes

@jberkhahn jberkhahn Awaiting requested review from jberkhahn

@joelanford joelanford Awaiting requested review from joelanford

Assignees

@jmrodri jmrodri

@estroz estroz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@camilamacedo86 @jmrodri @jberkhahn @johananl @openshift-ci-robot @estroz