Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

Replaced Godeps with glide #632

Merged
merged 5 commits into from
Jul 7, 2016
Merged

Replaced Godeps with glide #632

merged 5 commits into from
Jul 7, 2016

Conversation

tboerger
Copy link
Contributor

The same as for the other appc repositories. I have entirely replaced Godeps with glide and while I have imported Godeps into glide it still updated some dependencies.

@jonboulle
Copy link
Contributor

Same as elsewhere - this imports a bunch of cruft - could you run glide-vc please?

@tboerger
Copy link
Contributor Author

Same as elsewhere - this imports a bunch of cruft - could you run glide-vc please?

Did that now, thousands of changes lesser now.

@jonboulle
Copy link
Contributor

how about with the --no-tests flag? :-)

@tboerger
Copy link
Contributor Author

glide-vc --no-tests --only-code is a working combination. Applied it.

@s-urbaniak
Copy link
Contributor

@tboerger thanks! Just a few nitpicks from my side:

  • travis is failing, because the build script was renamed to build.sh in your PR.
  • Do you mind to add a glide-update.sh script, so others can bump dependencies in a reproducible way (see https://github.com/coreos/rkt/blob/master/scripts/glide-update.sh)?
  • I am wondering if we should document the glide update process, maybe in CONTRIBUTING.md?

@tboerger
Copy link
Contributor Author

tboerger commented Jul 5, 2016

Except the documentation on CONTRIBUTING.md I have applied the comments.

@jonboulle
Copy link
Contributor

@s-urbaniak rereview?

package: "github.com/appc/spec"
import:
- package: github.com/coreos/go-semver
version: 6fe83ccda8fb9b7549c9ab4ba47f47858bc950aa
Copy link
Contributor

Choose a reason for hiding this comment

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

"v0.1.0"

@s-urbaniak
Copy link
Contributor

@tboerger added a few version pointers for the glide.yaml, where available. We bumped to k8s 1.3.0 in master, so you'll have to revendor and rebase.

@tboerger
Copy link
Contributor Author

tboerger commented Jul 6, 2016

@s-urbaniak I think now it should be fine :)

@s-urbaniak
Copy link
Contributor

great, thanks, hopefully you won't kill me now if I ask you to reformat the commit messages to comply to https://github.com/appc/spec/blob/master/CONTRIBUTING.md#format-of-the-commit-message ;-)

@tboerger
Copy link
Contributor Author

tboerger commented Jul 6, 2016

@s-urbaniak hopefully you are happy now? :)

Thomas Boerger added 2 commits July 6, 2016 23:08
In preparation of the switch to glide instead of Godeps I have dropped
the entire Godeps directory.
I have generated the glide configuration to properly replace Godeps with
glide as a dependency manager.
@s-urbaniak
Copy link
Contributor

@tboerger good stuff, I checked out the PR locally, and found one thing, in commit 29e4e7a you're pulling all vendored ~140MB dependencies, and in commit f824d7b you are pruning them down to ~1MB.

I'd suggest to squash both commits (check in only pruned dependencies using glide-vc), that way we don't leave ~140MB of unnecessary blobs in the git history.

Other than that lgtm, and thanks a lot for the patience ;-)

Thomas Boerger added 3 commits July 7, 2016 15:28
I have added the vendored dependencies managed with glide, this works
only for the Go vendoring experiment of course.
I have unified the build and test script, now it should be the same
style as for all other appc projects.
To make updates of the dependencies easier I have added this script, now it's
just a single command to update the deps and clean unused packages.
@tboerger
Copy link
Contributor Author

tboerger commented Jul 7, 2016

@s-urbaniak done :)

@s-urbaniak
Copy link
Contributor

@tboerger thanks a lot, lgtm! 👍

@jonboulle
Copy link
Contributor

Outstanding. Thanks so much!

@jonboulle jonboulle merged commit 66a50da into appc:master Jul 7, 2016
@tboerger tboerger deleted the feature/vendoring branch July 7, 2016 15:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants