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

Upgrade graphsync deps #7598

Merged
merged 1 commit into from
Aug 18, 2020
Merged

Upgrade graphsync deps #7598

merged 1 commit into from
Aug 18, 2020

Conversation

ribasushi
Copy link
Contributor

There was supposed to be a bit more work landing here, but I won't get to it in time this week.
Let's unblock ipfs/go-graphsync#82 for now, maybe will have the more-involved go-car fix for rc2

)

replace github.com/ipfs/go-ipfs => ./../../..
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. Obviously we need this for now (i.e. until the go-ipfs 0.7.0 release) in order to get things to work, but should we just keep this replace directive here since it's an example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In e.g. go-car it is permanent: https://github.com/ipld/go-car/blob/v0.1.0/car/go.mod#L11
I don't mind either way, we can even remove it right now... ( there are no tests against it )

Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

Deps look upgraded and tests are passing so LGTM 😄. What's the status of a go-car release?

@@ -55,7 +55,7 @@ require (
github.com/ipfs/go-unixfs v0.2.4
github.com/ipfs/go-verifcid v0.0.1
github.com/ipfs/interface-go-ipfs-core v0.4.0
github.com/ipld/go-car v0.1.0
github.com/ipld/go-car v0.1.1-0.20200429200904-c222d793c339
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a release of go-car we can use here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is what I was referring to in my original comment: go-car needs a bit more work for a release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aschmahmann this is go-car master fwiw, so you are not depending on some random commit. I need to sneak in a particular fix in ( no API changes ), and that's another day or 2 away.

Copy link
Contributor

Choose a reason for hiding this comment

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

So should I be merging this and then we can update go-car later or am I waiting to merge on a go-car release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's merge this now, I won't be ready in a reasonable timeframe.

@aschmahmann aschmahmann merged commit 40738e4 into master Aug 18, 2020
@aschmahmann aschmahmann mentioned this pull request Sep 22, 2020
72 tasks
@hacdias hacdias deleted the chore/bump-graphsync-dep branch May 9, 2023 11:00
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.

2 participants