-
Notifications
You must be signed in to change notification settings - Fork 59
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
Update to unified graphsync v0.9.0 #615
Conversation
Codecov Report
@@ Coverage Diff @@
## master #615 +/- ##
==========================================
- Coverage 65.36% 65.27% -0.08%
==========================================
Files 62 62
Lines 4459 4459
==========================================
- Hits 2914 2910 -4
- Misses 1264 1268 +4
Partials 281 281
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is effectively a sed 's///'
PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like simple interface change adaptation. LGTM, but @aarshkshah1992 and @dirkmc should sign off too.
@@ -27,7 +27,7 @@ commands: | |||
condition: << parameters.linux >> | |||
steps: | |||
- run: sudo apt-get update | |||
- run: sudo apt-get install ocl-icd-opencl-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed now? Is it due to the FFI upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turned out didn't need ffi update so removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just kidding now I remember I had to do this in order to compile the FFI for M1 :(
go.mod
Outdated
github.com/filecoin-project/go-ds-versioning v0.1.0 | ||
github.com/filecoin-project/go-fil-commcid v0.1.0 | ||
github.com/filecoin-project/go-fil-commp-hashhash v0.1.0 | ||
github.com/filecoin-project/go-multistore v0.0.3 | ||
github.com/filecoin-project/go-multistore v0.0.4-0.20210601185713-428a2691a567 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we should definitely get rid of this.
go.mod
Outdated
@@ -7,11 +7,11 @@ require ( | |||
github.com/filecoin-project/go-address v0.0.5 | |||
github.com/filecoin-project/go-cbor-util v0.0.0-20191219014500-08c40a1e63a2 | |||
github.com/filecoin-project/go-commp-utils v0.1.1-0.20210427191551-70bf140d31c7 | |||
github.com/filecoin-project/go-data-transfer v1.7.6 | |||
github.com/filecoin-project/go-data-transfer v1.7.8-0.20210827051445-c354689456a5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flagging that this should change to go-data-transfer v1.8.0 when the corresponding PR is merged and released upstream.
cae978c
to
79056fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 👍
79056fd
to
9cea3e7
Compare
Update to go-graphsync v0.8.0 with go-ipld-prime linksystem branch & trusted store.
9cea3e7
to
79c52b7
Compare
Goals
This is based on #552, but rebased on master and updated to go-graphsync v0.9.0 which incorporates all the changes from the v0.6.x branch. It is part of the effort to unify graphsync versions post M1
Blocked by: