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

Run dep ensure -update #5988

Closed
wants to merge 3 commits into from
Closed

Run dep ensure -update #5988

wants to merge 3 commits into from

Conversation

oomichi
Copy link
Member

@oomichi oomichi commented Dec 15, 2017 •
edited
Loading

dep ensure never came back over 20mins on test-infra repo.
This fixes the issue by operating dep ensure -update and
hack/update-bazel.sh.

fixes #5987

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 15, 2017
@oomichi oomichi changed the title Run dep ensure -update WIP: Run dep ensure -update Dec 15, 2017
@k8s-ci-robot k8s-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 Dec 15, 2017
@BenTheElder
Copy link
Member

/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 Dec 15, 2017
@oomichi oomichi changed the title WIP: Run dep ensure -update Run dep ensure -update Dec 15, 2017
@k8s-ci-robot k8s-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 Dec 15, 2017
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: oomichi
We suggest the following additional approver: bentheelder

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

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

@oomichi oomichi force-pushed the dep-ensure-update branch 4 times, most recently from 4e217f1 to 960ec86 Compare December 16, 2017 01:41
@oomichi oomichi requested a review from cjwagner as a code owner December 16, 2017 01:41
Run dep ensure -update
bb55455 
`dep ensure` never came back over 20mins on test-infra repo.
This fixes the issue by operating `dep ensure -update` and
`hack/update-bazel.sh`.
Follow github changes after updating
ff51904 
`go vet` fails due to github library changes::

k8s.io/test-infra/ghclient
 ghclient/core.go:60:14: cannot use client.Repositories
   (type *github.RepositoriesService) as type repositoryService
   in field value:
 *github.RepositoriesService does not implement repositoryService
   (wrong type for ListCollaborators method)
   have ListCollaborators(context.Context, string, string, *github.ListCollaboratorsOptions)
      ([]*github.User *github.Response, error)
   want ListCollaborators(context.Context, string, string, *github.ListOptions)
      ([]*github.User, *github.Response error)

This patch makes ghclient follow these changes.
@stevekuznetsov
Copy link
Contributor

/assign @cblecker

@BenTheElder
Copy link
Member

This doesn't build currently due to mungegithub, do we really need the churn to update everything?

We should be moving mungegithub out to a graveyard or something in the near future but until then we probably don't want to do much with it.
/cc @cjwagner

Make mungegithub follow github changes
b5c7ba2 
This makes mungegithub follow github changes to avoid errors::

 mungegithub/github/github.go:723:29: cannot use
   prot.RequiredPullRequestReviews (type *github.PullRequestReviewsEnforcement)
   as type *github.PullRequestReviewsEnforcementRequest in field value
 mungegithub/github/github.go:1360:10: cannot use
   listOpts (type *github.ListOptions)
   as type *github.ListCollaboratorsOptions in argument to
   config.client.Repositories.ListCollaborators
@k8s-ci-robot
Copy link
Contributor

@oomichi: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-test-infra-verify-govet b5c7ba2 link /test pull-test-infra-verify-govet
pull-test-infra-bazel b5c7ba2 link /test pull-test-infra-bazel

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@oomichi
Copy link
Member Author

oomichi commented Dec 18, 2017

@BenTheElder

We should be moving mungegithub out to a graveyard or something in the near future
but until then we probably don't want to do much with it.

Thank you for your comment, I count not follow "graveyard" in the above comment, does it mean we can remove mungegithub from test-infra repo with moving the code to another repo? I cannot find "graveyard" repo, anyways.

@oomichi
Copy link
Member Author

oomichi commented Dec 18, 2017

I feel I see the point. Mungegithub itself is deprecated as the README.rst and we are migrating all mungers from mungegithub to prow1.
So we don't need to waste our time for maintaining mungegithub at this time.

TBH, I don't care of updating these code, I just want to import go-openapi as a new vendor code with dep ensure. If we can find another way/workaround except dep ensure -update that is good for me.

@cblecker
Copy link
Member

@oomichi have you tried a go get -u github.com/golang/dep/cmd/dep to update to the latest dep on your local environment? And then a dep ensure -add <import>? The other option is to remove dep from your system, and use the latest tagged binary (https://github.com/golang/dep/releases/tag/v0.3.2). I think this is a local env issue.

I'd suggest we close this PR out, and we can help diagnose your local env in #5987.

@BenTheElder
Copy link
Member

@oomichi yes, by "graveyard" I just mean deprecating / no longer working on this code.
@cblecker that sounds right to me. we probably just want a dep ensure -add here, our deps probably should get updated but a bit carefully.

@oomichi
Copy link
Member Author

oomichi commented Dec 19, 2017

@cblecker Yeah, it is nice to drop this PR and discuss the issue on the issue.
TBH, this change is so huge and I was a little afraid to do that.

@oomichi oomichi closed this Dec 19, 2017
@oomichi oomichi deleted the dep-ensure-update branch December 19, 2017 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjwagner cjwagner Awaiting requested review from cjwagner

Assignees

@BenTheElder BenTheElder

@cblecker cblecker

@cjwagner cjwagner

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dep ensure never comes back
6 participants
@oomichi @BenTheElder @k8s-ci-robot @stevekuznetsov @cblecker @cjwagner