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 vendored gengo #48630

Closed
wants to merge 2 commits into from
Closed

update vendored gengo #48630

wants to merge 2 commits into from

Conversation

nikhita
Copy link
Member

@nikhita nikhita commented Jul 7, 2017

Need to fix defaulter-gen for #47263.
To pull in changes from kubernetes/gengo#61.

Release note:

NONE

/cc @sttts

@k8s-ci-robot k8s-ci-robot requested a review from sttts July 7, 2017 19:53
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 7, 2017
@k8s-ci-robot
Copy link
Contributor

Hi @nikhita. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 7, 2017
@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Jul 7, 2017
nikhita
nikhita commented Jul 7, 2017
View reviewed changes
Godeps/Godeps.json Outdated
@@ -415,7 +416,7 @@
},
{
"ImportPath": "github.com/codegangsta/negroni",
"Comment": "v0.1-62-g8d75e11",
"Comment": "v0.1.0-62-g8d75e11",
Copy link
Member Author

Choose a reason for hiding this comment

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

@sttts ptal

@sttts sttts assigned sttts and unassigned smarterclayton and brendandburns Jul 7, 2017
@sttts
Copy link
Contributor

sttts commented Jul 7, 2017

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 7, 2017
nikhita
nikhita commented Jul 7, 2017
View reviewed changes
hack/godep-save.sh Outdated
@@ -30,6 +30,7 @@ REQUIRED_BINS=(
"github.com/ugorji/go/codec/codecgen"
"github.com/onsi/ginkgo/ginkgo"
"github.com/jteeuwen/go-bindata/go-bindata"
"k8s.io/gengo/examples/..."
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this. 👍

@nikhita nikhita force-pushed the defaulter-gen-vendor branch from 84606d1 to c33a42f Compare July 7, 2017 20:10
@sttts
Copy link
Contributor

sttts commented Jul 7, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2017
@nikhita nikhita force-pushed the defaulter-gen-vendor branch from c33a42f to 6acbc6f Compare July 7, 2017 20:46
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 7, 2017
@nikhita
Copy link
Member Author

nikhita commented Jul 7, 2017 •
edited
Loading

cc @lavalamp for approval

@k8s-github-robot k8s-github-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 Jul 8, 2017
@cblecker
Copy link
Member

cblecker commented Jul 8, 2017

verify is failing due to #48653

@nikhita
Copy link
Member Author

nikhita commented Jul 8, 2017

Thanks, @cblecker.

@cblecker
Copy link
Member

cblecker commented Jul 9, 2017

/retest

@nikhita nikhita force-pushed the defaulter-gen-vendor branch from 0fd882b to 222240b Compare July 9, 2017 03:58
@nikhita
Copy link
Member Author

nikhita commented Jul 9, 2017

/retest

@nikhita
Copy link
Member Author

nikhita commented Jul 9, 2017

The tests pass locally though:

➜  kubernetes git:(defaulter-gen-vendor) ./hack/verify-staging-client-go.sh                                  132s 
Smoke testing client-go examples
Checking whether godeps are restored
Running staging/copy.sh
  creating the tmp directory
  copying client packages
  rewriting imports
  rearranging directory layout
  running gofmt
  remove black listed files
  running FAIL_ON_CHANGES
  /home/nikhita/code/kubernetes/src/k8s.io/kubernetes/staging/src/k8s.io/client-go up to date.

@nikhita nikhita mentioned this pull request Jul 9, 2017
Closed
@nikhita
Copy link
Member Author

nikhita commented Jul 9, 2017 •
edited
Loading

Probably hitting #48675.

Running ./hack/update-codegen.sh did not update the auto-generated files at all places where it was necessary.

Removed the files that the CI complained of and regenerated them using make. They got updated now.

@nikhita
Copy link
Member Author

nikhita commented Jul 9, 2017

/retest

2 similar comments
@nikhita
Copy link
Member Author

nikhita commented Jul 9, 2017

/retest

@nikhita
Copy link
Member Author

nikhita commented Jul 9, 2017

/retest

@nikhita nikhita force-pushed the defaulter-gen-vendor branch from 38ac8a2 to ab26c93 Compare July 10, 2017 11:05
@sttts sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 10, 2017
@sttts
Copy link
Contributor

sttts commented Jul 10, 2017

@nikhita can you squash the last two commits?

@nikhita nikhita force-pushed the defaulter-gen-vendor branch from ab26c93 to 40fd2f5 Compare July 10, 2017 11:13
@nikhita
Copy link
Member Author

nikhita commented Jul 10, 2017

@sttts done.

@sttts sttts added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jul 10, 2017
@nikhita
Copy link
Member Author

nikhita commented Jul 10, 2017

/retest

1 similar comment
@nikhita
Copy link
Member Author

nikhita commented Jul 12, 2017

/retest

@sttts
Copy link
Contributor

sttts commented Jul 12, 2017

Can you rebase?

@sttts
Copy link
Contributor

sttts commented Jul 12, 2017

@smarterclayton approved?

@nikhita nikhita force-pushed the defaulter-gen-vendor branch from 40fd2f5 to 69a9af8 Compare July 12, 2017 08:01
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 12, 2017
@sttts
Copy link
Contributor

sttts commented Jul 12, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 12, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nikhita, sttts
We suggest the following additional approver: dchen1107

Assign the PR to them by writing /assign @dchen1107 in a comment when ready.

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot
Copy link

@nikhita PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2017
@nikhita
Copy link
Member Author

nikhita commented Jul 18, 2017

I don't think this PR is needed anymore. gengo has already been updated in #48544. Closing.

@nikhita nikhita closed this Jul 18, 2017
@nikhita nikhita deleted the defaulter-gen-vendor branch July 19, 2017 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts Awaiting requested review from sttts

Assignees

@sttts sttts

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nikhita @k8s-ci-robot @sttts @cblecker @k8s-github-robot @smarterclayton @brendandburns