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: add deps of binary tools/utils #11665

Merged
merged 1 commit into from
Nov 28, 2016
Merged

vendor: add deps of binary tools/utils #11665

merged 1 commit into from
Nov 28, 2016

Conversation

dt
Copy link
Member

@dt dt commented Nov 28, 2016

a couple of these got dropped when we recomputed deps from source, since they aren't transitive deps of cockroach,
only of tools we use


This change is Reviewable

@rjnn
Copy link
Contributor

rjnn commented Nov 28, 2016

Reproducing on a fresh gceworker, will update with the results.


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

a couple of these got dropped when we recomputed deps from source, since they aren't transitive deps of cockroach,
only of tools we use
@rjnn
Copy link
Contributor

rjnn commented Nov 28, 2016

vendor/github.com/robfig/glock/sync.go:11:2: cannot find package "github.com/agtorre/gocolorize" in any of:
/home/arjun/go/src/github.com/cockroachdb/cockroach/vendor/github.com/agtorre/gocolorize (vendor tree)
/home/arjun/goroot/src/github.com/agtorre/gocolorize (from $GOROOT)
/home/arjun/go/src/github.com/agtorre/gocolorize (from $GOPATH)


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

@vivekmenezes
Copy link
Contributor

LGTM

@vivekmenezes
Copy link
Contributor

I just newly fetched the code and those were the only three dependencies. LGTM

@tamird
Copy link
Contributor

tamird commented Nov 28, 2016

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


glide.yaml, line 13 at r1 (raw file):

- package: github.com/chzyer/readline
- package: github.com/cockroachdb/c-jemalloc
  # Inherently risky.

why remove these comments?


Comments from Reviewable

@dt
Copy link
Member Author

dt commented Nov 28, 2016

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


glide.yaml, line 13 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

why remove these comments?

They're auto-removed any time glide get or otherwise use glide to edit glide.yaml


Comments from Reviewable

@tamird
Copy link
Contributor

tamird commented Nov 28, 2016

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


glide.yaml, line 13 at r1 (raw file):

Previously, dt (David Taylor) wrote…

They're auto-removed any time glide get or otherwise use glide to edit glide.yaml 💩

The gift that keeps on giving.


Comments from Reviewable

@dt dt merged commit cdbaf3d into cockroachdb:master Nov 28, 2016
@dt dt deleted the deps branch November 28, 2016 20:28
@tamird
Copy link
Contributor

tamird commented Nov 28, 2016

Wait, so you decided not to restore those comments? Sigh.

@dt
Copy link
Member Author

dt commented Nov 29, 2016

@tamird filed an issue upstream about that, though looking at the lib they use, go-yaml, i'm not sure there's anyway to parse and capture comment nodes, so i'm not holding my breath.

@dt
Copy link
Member Author

dt commented Nov 29, 2016

Masterminds/glide#691

@dt
Copy link
Member Author

dt commented Nov 29, 2016

I also filed one for the more general issue of our vendored tools missing their transitive deps:
Masterminds/glide#690

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants