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

commands/.../generate/*: set generator header file if not supplied #1239

Merged
merged 5 commits into from
Mar 26, 2019
Merged

commands/.../generate/*: set generator header file if not supplied #1239

merged 5 commits into from
Mar 26, 2019

Conversation

estroz
Copy link
Member

@estroz estroz commented Mar 25, 2019

Description of the change: allow users to pass in a header file for generate k8s, or use a temp file if one is not supplied.

Motivation for the change: deepcopy-gen and defaulter-gen now require header files, and will fail if not provided. I noticed this when testing the SDK with go modules; go mod vendor removes default header files from the gengo repo.

@estroz estroz added the kind/bug Categorizes issue or PR as related to a bug. label Mar 25, 2019
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 25, 2019
@lilic
Copy link
Member

lilic commented Mar 26, 2019

go mod vendor removes default header files from the gengo repo.

Interesting, what is the main reason behind this behavior?

lilic
lilic approved these changes Mar 26, 2019
View reviewed changes
Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

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

lgtm

@estroz
Copy link
Member Author

estroz commented Mar 26, 2019

@lilic go mod vendor removes directories that do not contain source code from vendor/.

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 26, 2019
joelanford
joelanford approved these changes Mar 26, 2019
View reviewed changes
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

LGTM, with the one totally optional style nit 🙂

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 26, 2019
@estroz estroz merged commit 2761657 into operator-framework:master Mar 26, 2019
@estroz estroz deleted the gen-header-files branch March 26, 2019 21:31
@estroz estroz mentioned this pull request Mar 27, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelanford joelanford joelanford approved these changes

@lilic lilic lilic approved these changes

@AlexNPavel AlexNPavel Awaiting requested review from AlexNPavel

@hasbro17 hasbro17 Awaiting requested review from hasbro17

@theishshah theishshah Awaiting requested review from theishshah

Assignees
No one assigned
Labels
kind/bug Categorizes issue or PR as related to a bug. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@estroz @lilic @joelanford @openshift-ci-robot