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

vendor: Use dep instead of glide and prune it #557

Merged
merged 4 commits into from
Apr 26, 2018
Merged

vendor: Use dep instead of glide and prune it #557

merged 4 commits into from
Apr 26, 2018

Conversation

gaocegege
Copy link
Member

@gaocegege gaocegege commented Apr 24, 2018

  • Use dep instead of glide
  • Remove unused packages from vendor tree via dep prune -v +27,065 −1,698,564
  • Add code-generator in vendor and regenerate

/assign @zjj2wry @jlewi

Close #556


This change is Reviewable

@gaocegege
Copy link
Member Author

gaocegege commented Apr 24, 2018

The first two commits are auto generated. 27d46f1 and 1bc19e5 are manually changed by me. PTAL

@coveralls
Copy link

Coverage Status

Coverage decreased (-11.4%) to 38.951% when pulling 1bc19e5 on gaocegege:dep into 60d25c4 on kubeflow:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-11.4%) to 38.951% when pulling 1bc19e5 on gaocegege:dep into 60d25c4 on kubeflow:master.

@coveralls
Copy link

coveralls commented Apr 24, 2018

Coverage Status

Coverage decreased (-0.08%) to 49.4% when pulling 0a752d3 on gaocegege:dep into 07fd2d6 on kubeflow:master.

@gaocegege
Copy link
Member Author

The error in Travis CI is fixed in #548

@jlewi
Copy link
Contributor

jlewi commented Apr 24, 2018

Why switch to dep instead of glide? We've been using glide in most places I think.

/cc @wbuchwalter

@gaocegege
Copy link
Member Author

gaocegege commented Apr 24, 2018

Because of #541

As you see, we reduced the code base significantly.

+ 27,065
- 1,698,564

Besides this, dep is more popular and avtive than glide: https://go.libhunt.com/compare-dep-vs-glide

@jlewi
Copy link
Contributor

jlewi commented Apr 24, 2018

I don't have a strong oppinion but as I commented on the issue lets try to get consensus and adopt a single tool for all of Kubeflow.

Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
@gaocegege
Copy link
Member Author

Per the discussion #556 , we come to an agreement. And I solved the problem in Travis, PTAL @jlewi @ScorpioCPH @zjj2wry

@zjj2wry
Copy link
Member

zjj2wry commented Apr 25, 2018

/lgtm thank you do this ~

@gaocegege
Copy link
Member Author

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gaocegege, zjj2wry

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

@gaocegege
Copy link
Member Author

It seems that my approve does not work

/assign @jlewi @ScorpioCPH

@gaocegege gaocegege deleted the dep branch May 28, 2018 05:56
yph152 pushed a commit to yph152/tf-operator that referenced this pull request Jun 18, 2018
* vendor: Update

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* pkg: Regenerate

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* update-codegen: Update

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* vendor: Replace glide

Signed-off-by: Ce Gao <gaoce@caicloud.io>
jetmuffin pushed a commit to jetmuffin/tf-operator that referenced this pull request Jul 9, 2018
* vendor: Update

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* pkg: Regenerate

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* update-codegen: Update

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* vendor: Replace glide

Signed-off-by: Ce Gao <gaoce@caicloud.io>
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.

6 participants