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.

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

Add Gopkg dependency for kubernetes code-generator. #371

Merged
merged 1 commit into from
Nov 30, 2018

Conversation

neuromage
Copy link
Contributor

@neuromage neuromage commented Nov 26, 2018

The code generator should not be run from HEAD, as it will generate code
that diverges from the pinned version of client-go, and also any
previously generated CRD controller clients.

This change pins both code generator and client-go to the specified
kubernetes release, and ensures the update-codegen.sh script uses the
code-generator specified in the vendor directory rather than HEAD. This
ensures the build is always reproducible.

Note: I made changes to Gopkg.toml and the update-codegen.sh script. The changes in vendor/ is the result of running dep ensure.


This change is Reviewable

@neuromage neuromage requested review from vicaire and removed request for qimingj November 26, 2018 01:41
vendor/OWNERS Outdated
@@ -1,10 +0,0 @@
approvers:
Copy link
Member

Choose a reason for hiding this comment

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

is this change intentional? if not is there any way to persist this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running dep ensure will delete that file, since all contents under vendor are assumed to be generated. I'm not sure how to keep that file in. Also I'm not sure if there's a benefit of having an OWNERS file in the vendor directory is, since changes under this directory should be a side effect of code, rather than a code change itself. What do you think?

@vicaire
Copy link
Contributor

vicaire commented Nov 29, 2018

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vicaire

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@neuromage
Copy link
Contributor Author

/test build-image

The code generator should not be run from HEAD, as it will generate code
that diverges from the pinned version of client-go, and also any
previously generated CRD controller clients.

This change pins both code generator and client-go to the specified
kubernetes release, and ensures the update-codegen.sh script uses the
code-generator specified in the vendor directory rather than HEAD. This
ensures the build is always reproducible.
@neuromage
Copy link
Contributor Author

@vicaire and @IronPan can you take another look? I messed up some merges, which removed the lgtm, but finally managed to fix everything. PTAL thanks.

@IronPan
Copy link
Member

IronPan commented Nov 30, 2018

/lgtm

@neuromage
Copy link
Contributor Author

/retest

1 similar comment
@neuromage
Copy link
Contributor Author

/retest

@neuromage
Copy link
Contributor Author

/test presubmit-e2e-test

@k8s-ci-robot k8s-ci-robot merged commit 8a13287 into master Nov 30, 2018
@IronPan IronPan deleted the neuromage/crd-dependencies branch June 28, 2019 18:48
Linchin pushed a commit to Linchin/pipelines that referenced this pull request Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants