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

INSTALL.md: update Glide instructions to prefer glide update #127

Merged
merged 1 commit into from
Feb 23, 2017

Conversation

ericchiang
Copy link
Contributor

glide get has weird dependencies on the glide cache.
glide install just updates glide.lock but doesn't update /vendor.
glide init will detect dependencies from your current project,
making it dependent on the external files.

Just recommend users create a glide.yaml manually, then use
glide update to populate their /vendor directory. Technically you
could do a dance of glide init -> glide get -> glide update[0]
but this seems less error prone.

Additionally, provide instructions for users who want to use
different versions of packages than client-go, and add a note
about glide-vc.

[0] https://github.com/coreos/etcd/blob/v3.1.1/scripts/updatedep.sh

Sorry, client-go does not accept changes via pull requests at this time. Please
submit your pull request to the main repository:
https://github.com/kubernetes/kubernetes. See the guidance here:
https://github.com/kubernetes/client-go#contributing-code.

cc @caesarxuchao @lavalamp @kubernetes/client-go-reviewers

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 22, 2017
@k8s-reviewable
Copy link

This change is Reviewable

INSTALL.md Outdated
version: v2.0.0
```

Second, add a Go file that imports `client-go` somewhere in your project (Glide
Copy link
Member

Choose a reason for hiding this comment

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

I tried it locally. Glide still added client-go to vendor/, but it didn't parse client-go/godeps.json, so all client-go's dependencies were missed in vendor/. So how about this:

Second, add a Go file that imports client-go somewhere in your project, otherwise client-go's dependencies will not be added to your procject's vendor/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, thanks!

INSTALL.md Outdated
glide up -v
```

Importantly, Glide will detect the versions of dependencies `client-go`
Copy link
Member

Choose a reason for hiding this comment

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

Could you describe the directory layout after the second step is done? Maybe something like this:

At this point, k8s.io/client-go should be added to your project's vendor/. client-go's dependencies should be flattened and be added to your project's vendor/ as well.

Glide will detect the versions of dependencies client-go specified in client-go's Godep.json file, and automatically set the versions of these imports in your /vendor directory. It will also record the detected version of all dependencies in the glide.lock file.

INSTALL.md Outdated
```

Importantly, Glide will detect the versions of dependencies `client-go`
specifies in its `Godep.json` file, and automatically set the versions of these
Copy link
Member

Choose a reason for hiding this comment

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

s/specifies/specified

INSTALL.md Outdated
imports in your /vendor directory. It will also record the detected version of
all dependencies in the `glide.lock` file.

Glide will prefer package versions specified in `glide.yaml` over versions
Copy link
Member

Choose a reason for hiding this comment

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

Could you make it clear that the setup is done after step 2, and this paragraph is optional, only useful if the user wants to overwrite the version of the dep?

@caesarxuchao
Copy link
Member

Left a couple more comments. Thanks for the pull!

FYI @lavalamp

`glide get` has weird dependencies on the glide cache.
`glide install` just updates `glide.lock` but doesn't update /vendor.
`glide init` will detect dependencies from your current project,
making it dependent on the external files.

Just recommend users create a `glide.yaml` manually, then use
`glide update` to populate their /vendor directory. Technically you
could do a dance of `glide init` -> `glide get` -> `glide update`[0]
but this seems less error prone.

Additionally, provide instructions for users who want to use
different versions of packages than `client-go`, and add a note
about glide-vc.

[0] https://github.com/coreos/etcd/blob/v3.1.1/scripts/updatedep.sh
imports in your /vendor directory. It will also record the detected version of
all dependencies in the `glide.lock` file.

Projects that require a different version of a dependency than `client-go`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@caesarxuchao this work?

@caesarxuchao
Copy link
Member

Fix #122.

@caesarxuchao
Copy link
Member

lgtm.

@ash2k @lvangool @jaxxstorm @lavalamp it would be great if you can try the new guide in your environment :)

@lvangool
Copy link

@caesarxuchao confirmed new guide is working. LGTM!

@caesarxuchao
Copy link
Member

Cool. Merging. Thank you all.

@caesarxuchao caesarxuchao merged commit 5fbce75 into kubernetes:master Feb 23, 2017
@ericchiang ericchiang deleted the glide-instructions branch February 23, 2017 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants