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

Update to unified go-graphsync v0.9.0 #7197

Merged
merged 3 commits into from
Sep 6, 2021
Merged

Conversation

hannahhoward
Copy link
Contributor

@hannahhoward hannahhoward commented Aug 27, 2021

Goals

This is based on #6375, 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:

@hannahhoward hannahhoward requested a review from a team as a code owner August 27, 2021 06:13
@codecov
Copy link

codecov bot commented Aug 27, 2021

Codecov Report

Merging #7197 (dc1f482) into master (212400b) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7197      +/-   ##
==========================================
- Coverage   39.09%   39.08%   -0.01%     
==========================================
  Files         614      614              
  Lines       64895    64892       -3     
==========================================
- Hits        25369    25362       -7     
- Misses      35127    35128       +1     
- Partials     4399     4402       +3     
Impacted Files Coverage Δ
node/modules/graphsync.go 63.15% <100.00%> (-3.51%) ⬇️
node/modules/storageminer.go 62.07% <100.00%> (-0.07%) ⬇️
markets/loggers/loggers.go 89.28% <0.00%> (-10.72%) ⬇️
extern/sector-storage/manager_calltracker.go 57.70% <0.00%> (-4.85%) ⬇️
chain/stmgr/call.go 67.87% <0.00%> (-3.64%) ⬇️
node/hello/hello.go 63.63% <0.00%> (-3.41%) ⬇️
itests/kit/blockminer.go 93.65% <0.00%> (-1.59%) ⬇️
chain/vm/vm.go 59.47% <0.00%> (-1.12%) ⬇️
chain/sync.go 64.94% <0.00%> (-0.34%) ⬇️
chain/sync_manager.go 67.08% <0.00%> (-0.32%) ⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 212400b...dc1f482. Read the comment docs.

@jennijuju jennijuju requested a review from raulk August 27, 2021 18:24
Copy link
Collaborator

@ribasushi ribasushi left a 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
:shipit:

@Stebalien Stebalien force-pushed the feat/update-graphsync-v0.9.0 branch 2 times, most recently from 85d3741 to 28e9ffb Compare August 27, 2021 22:11
Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

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

Looks like we have some compile errors due to upstream dependency incompatibilities?

@dirkmc
Copy link
Contributor

dirkmc commented Aug 31, 2021

Approved modulo updating deps 👍

@hannahhoward hannahhoward force-pushed the feat/update-graphsync-v0.9.0 branch from 28e9ffb to 92c84f2 Compare September 1, 2021 23:35
@jennijuju jennijuju requested a review from raulk September 2, 2021 00:11
@jennijuju jennijuju added the P1 P1: Must be resolved label Sep 2, 2021
@dirkmc
Copy link
Contributor

dirkmc commented Sep 2, 2021

Note: Please do not merge until we've had a chance to run a manual test

Update: manual tests complete

@dirkmc dirkmc force-pushed the feat/update-graphsync-v0.9.0 branch from f342832 to 19883d9 Compare September 3, 2021 14:21
hannahhoward and others added 3 commits September 3, 2021 16:21
Update to go-graphsync v0.8.0 with go-ipld-prime linksystem branch & trusted store.
@dirkmc dirkmc force-pushed the feat/update-graphsync-v0.9.0 branch from 19883d9 to dc1f482 Compare September 3, 2021 14:23
@dirkmc
Copy link
Contributor

dirkmc commented Sep 3, 2021

Manual tests completed.
Let's hold off on merging until we've had a chance to test with miner x cohort.

Manually completed storage and retrieval deals for

  • graphsync-v0.9 client <-> master server
  • master server <-> graphsync-v0.9 client
  • graphsync-v0.9 client <-> graphsync-v0.9 server

@dirkmc
Copy link
Contributor

dirkmc commented Sep 6, 2021

Miner x cohort approved ✅
This PR is ready for merge

@jennijuju jennijuju merged commit e922791 into master Sep 6, 2021
@jennijuju jennijuju deleted the feat/update-graphsync-v0.9.0 branch September 6, 2021 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 P1: Must be resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants