Skip to content

Remove unused Go operator scaffold code#3249

Merged
hasbro17 merged 1 commit intooperator-framework:masterfrom
hasbro17:remove-unused-go-scaffold-code
Jun 17, 2020
Merged

Remove unused Go operator scaffold code#3249
hasbro17 merged 1 commit intooperator-framework:masterfrom
hasbro17:remove-unused-go-scaffold-code

Conversation

@hasbro17
Copy link
Copy Markdown
Contributor

Description of the change:
Remove unused scaffolding code for Go operators.

Motivation for the change:
Follow up from #3190 (review)

@hasbro17 hasbro17 added the kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form label Jun 17, 2020
@hasbro17
Copy link
Copy Markdown
Contributor Author

Ah whoops, removed too much.
Going to add back the bits that we actually still need.

@hasbro17 hasbro17 changed the title Remove unused Go operator scaffold code WIP: Remove unused Go operator scaffold code Jun 17, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 17, 2020
@camilamacedo86
Copy link
Copy Markdown
Contributor

yes, we still need the scaffold used in the add command. otherwise, it shows good.

@hasbro17 hasbro17 force-pushed the remove-unused-go-scaffold-code branch from 9f33c16 to 37947cd Compare June 17, 2020 19:10
@hasbro17
Copy link
Copy Markdown
Contributor Author

Turns out pretty much everything in internal/scaffold is still being used by one cmd or the other in the legacy CLI.
So not a whole lot here until that gets removed.

@hasbro17 hasbro17 changed the title WIP: Remove unused Go operator scaffold code Remove unused Go operator scaffold code Jun 17, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 17, 2020
Copy link
Copy Markdown
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

Thoughts on making a kubebuilder PR for internal/scaffold/version.go? Its a nice-to-have for operators.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2020
@hasbro17
Copy link
Copy Markdown
Contributor Author

@estroz I'm not opposed but I imagine scaffolding a version/version.go for operator projects is one of those things that Kubebuilder probably won't want to be opinionated about since it doesn't have much to do with the workflow of writing controllers/operators.

Plus other projects in the wild each have their own preference of where to place the version information.
https://github.com/jetstack/cert-manager/blob/master/cmd/ctl/pkg/version/version.go
https://github.com/coreos/prometheus-operator/blob/master/VERSION
https://github.com/oracle/mysql-operator/blob/master/pkg/version/version.go
https://github.com/zalando/postgres-operator/blob/master/kubectl-pg/cmd/version.go
https://github.com/rook/rook/blob/master/pkg/version/version.go#L19
https://github.com/vmware-tanzu/velero/blob/master/pkg/buildinfo/version.go#L26

So I imagine that might be out of scope for Kubebuilder.

@hasbro17 hasbro17 merged commit 0faa762 into operator-framework:master Jun 17, 2020
@hasbro17 hasbro17 deleted the remove-unused-go-scaffold-code branch June 17, 2020 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants