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

Godeps maintenance #1156

Merged
merged 5 commits into from
Apr 29, 2015
Merged

Godeps maintenance #1156

merged 5 commits into from
Apr 29, 2015

Conversation

cryptix
Copy link
Contributor

@cryptix cryptix commented Apr 28, 2015

Main motivation was to have everything at latest master and to move away from code.google.com/p

  • moved (go)goprotobuf to github location

  • added iptb to the sadhack package - 'make vendor' can't see it otherwise

  • Followed up remove debugerrors #1098 so that all its code is dropped

    I also experimented with the eg tool some time ago. Want to do that again.

  • updated all dependencies to latest master

  • hashicorp/mdns uses a fork of go.net which isn't up to date - issue

  • we still use go-uuid from code.google.com in Godeps/.../github.com/jbenet/go-datastore/key.go and ./thirdparty/eventlog/metadata.go. There are dozens of forks and other versions. Maybe we can find one with an identical interface and drop that in.

I think we can merge this now and wait for hashicorp/mdns#37 to proceed with the rest.

@jbenet jbenet added the status/in-progress In progress label Apr 28, 2015
@jbenet
Copy link
Member

jbenet commented Apr 28, 2015

This LGTM. sounds good about merging now.

@jbenet
Copy link
Member

jbenet commented Apr 28, 2015

Dont really care what we do about uuid -- whatever works.

in general I'm all for staying up to date (cause bugs get fixed) though sometimes, newer isn't always better -- sometimes newer releases are actually more broken. We should always verify that the changelogs show improvement. (thoughts on these @cryptix ?)

@wking
Copy link
Contributor

wking commented Apr 28, 2015

On Tue, Apr 28, 2015 at 10:41:37AM -0700, Juan Batiz-Benet wrote:

We should always verify that the changelogs show improvement.

I don't know anything about our dependencies here, but if they are
cutting versioned releases it would be nice to bump our vendored
version to their latest point release, instead of their latest master.
That might also make finding changelogs/release-notes easier to figure
out what has changed.

@jbenet
Copy link
Member

jbenet commented Apr 28, 2015

yeah, point releases may be a safer strategy.

@jbenet
Copy link
Member

jbenet commented Apr 28, 2015

@dylanPowers it may be useful to compile a lot of these workflow preferences (like how to vendor, what to vendor, requirement that all commits pass, etc) into some doc in community repo.

@cryptix
Copy link
Contributor Author

cryptix commented Apr 29, 2015

in general I'm all for staying up to date (cause bugs get fixed) though sometimes, newer isn't always better -- sometimes newer releases are actually more broken. We should always verify that the changelogs show improvement. (thoughts on these @cryptix ?)

re things can get worse: Agreed, but tracking the changelog of every package down our import tree is a lot of work. IMHO we should classify our dependencies and act accordingly - at least by direct and indirect imports.

For instance, currently our most fragile dependencies are bazil.org/fuse, github.com/syndtr/goleveldb/leveldb, gogoproto/... and github.com/hashicorp/yamux, I would say. Breaking changes to these are dangerous, afaict the rest is mostly utility and imports by other imports.

Personally, I would like to do this: Move everything to latest (maybe bi-)weekly, dog food our tests to catch regressions (we should also deploy benchcmp on our next gen ci) and deploy a 'staging beta' test network of maybe 10 nodes with the newest version of the deps, running profiling for a week. If it's green push the updates into the next release.

For updating to latest master I'd use these rules:

  1. indirect imports stay at the version that the 'parent' wanted.
  2. direct import with a version system should just bump minor and patch versions and have good reasons for a major upgrade.
  3. direct imports without a version just get bumped to master. (and maybe a nagging comment in their issue tracker, depending on the size of the dep)

All of these defiently exceeds my current Go-Package-Store && godep update... workflow which is a bit foolish but to be honest, I think it's very important to keep an eye on the dependencies and not loose track of them, especially in alpha stage where I feel that 'We finished these 5 features over the last x month but in the meantime 2 critical layers of dependency started melting and converging and now we have a lot of adoption and replacement 💩 on our hands'.

That beeing said, I think the writing is on the wall for a proper godeps replacement. And I feel like we should build the tools we need for it. Maybe also look at shurcool's gostatus to automate upstream change monitoring.

edit: the critical list is longer, of course, x/net/context, x/crypto/..., etc.. wasn't meant to be complete

@jbenet
Copy link
Member

jbenet commented Apr 29, 2015

I'm down to start making the tools we need.

import (
   pkgname "ipfs.io/ipfs/<hash>"
)

@jbenet
Copy link
Member

jbenet commented Apr 29, 2015

on the rest-- i think in general the deps tend to either be stable or fail horribly (compiler errors). subtle bugs are way rarer. dont want to add a ton of workflow barriers to updating deps (e.g. pushing to some nodes and waiting for a week). this probably is what we want to do down the road for safety, but right now would slow us down.

i'll merge this

jbenet added a commit that referenced this pull request Apr 29, 2015
@jbenet jbenet merged commit cc5f6bb into master Apr 29, 2015
@jbenet jbenet removed the status/in-progress In progress label Apr 29, 2015
@jbenet jbenet deleted the maintenance branch April 29, 2015 19:28
@cryptix cryptix mentioned this pull request Oct 26, 2015
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