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

Fix godep restore #761

Closed
wants to merge 2 commits into from
Closed

Fix godep restore #761

wants to merge 2 commits into from

Conversation

chriscool
Copy link
Contributor

No description provided.

@btc btc added the status/in-progress In progress label Feb 7, 2015
@chriscool
Copy link
Contributor Author

It would be nice to have an automatic test that does "cd test/dependencies && make" to avoid these problems.

@chriscool
Copy link
Contributor Author

The "race" Jenkins test failed with:

ok      github.com/jbenet/go-ipfs/p2p/protocol/relay    23.138s
--- FAIL: TestStBackpressureStreamWrite (11.00 seconds)
    backpressure_test.go:343: continuous should have been faster
FAIL
FAIL    github.com/jbenet/go-ipfs/p2p/test/backpressure 11.029s

but it works locally on my machine.

@jbenet
Copy link
Member

jbenet commented Feb 7, 2015

@chriscool

  • apologies for the timing dependent tests failing on CI. we really need to fix this.
  • and also sorry, i hadnt pushed multiformats/go-multiaddr@c13f11b
  • @briantigerchow did you push inflect?
  • yes, we should add a test that makes sure all the other repos are accessible.

@chriscool I'm curious, why do you need to godep restore? (so that make vendor works?)

@chriscool
Copy link
Contributor Author

@jbenet yeah I want to use a recent version of go-multihash so that we can use the multihash tool instead of shasum. Right now we use multiformats/go-multihash@8ce5cb1 which is old.

@jbenet
Copy link
Member

jbenet commented Feb 7, 2015

I'm also at briantigerchow/inflect@90ecbd7 -- the diff I see with what's in the godep workspace is:

> diff -r ../../briantigerchow/inflect Godeps/_workspace/src/github.com/briantigerchow/inflect
Only in ../../briantigerchow/inflect: .git
diff -r ../../briantigerchow/inflect/languages/english.go Godeps/_workspace/src/github.com/briantigerchow/inflect/languages/english.go
5c5
<   "github.com/chuckpreslar/inflect/types"
---
>   "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/briantigerchow/inflect/types"
diff -r ../../briantigerchow/inflect/languages.go Godeps/_workspace/src/github.com/briantigerchow/inflect/languages.go
5,6c5,6
<   "github.com/chuckpreslar/inflect/languages"
<   "github.com/chuckpreslar/inflect/types"
---
>   "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/briantigerchow/inflect/languages"
>   "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/briantigerchow/inflect/types"

So yeah, let's merge the hash update you give. though without the multiaddr hash update (that was my mistake)

@jbenet
Copy link
Member

jbenet commented Mar 2, 2015

I think inflect has changed at this point, and multiaddr was fine. I think the PR is not needed anymore? thanks though @chriscool

It would be nice to have an automatic test that does "cd test/dependencies && make" to avoid these problems.

Yes!! it really would be nice. And (as you had mentioned) one that perhaps godep restores so people cannot godep vendor without pushing to the world (does not guarantee that the code will remain there, but at least it will handle 80% of failures).

@jbenet
Copy link
Member

jbenet commented Mar 2, 2015

pls reopen if i'm mistaken in thinking this is no longer necessary.

@jbenet jbenet closed this Mar 2, 2015
@jbenet jbenet removed the status/in-progress In progress label Mar 2, 2015
@chriscool
Copy link
Contributor Author

You are right, it is not necessary any more.

@chriscool chriscool deleted the fix_godep_restore branch April 30, 2016 05:48
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.

3 participants