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.

Sign up for GitHub

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

Jump to bottom

use go-padreader instead of local copy #103

Merged
merged 1 commit into from
Jan 30, 2020
Merged

use go-padreader instead of local copy #103

merged 1 commit into from
Jan 30, 2020

Conversation

laser
Copy link
Contributor

@laser laser commented Jan 30, 2020

I've extracted go-padreader into a new repo so that it can be shared by other projects (lotus, go-storage-miner, and this project).

This changeset backports that extraction into go-fil-markets.

@laser laser force-pushed the chores/consume-extracted-padreader branch from 1681e65 to c32d45f Compare January 30, 2020 21:00
@laser laser requested a review from ingar January 30, 2020 21:00
ingar
ingar approved these changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

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

Looks good. There is a PadReader.go generated mock that we should probably get rid of also.

ingar
ingar reviewed Jan 30, 2020
View reviewed changes
go.sum Outdated Show resolved Hide resolved
hannahhoward
hannahhoward approved these changes Jan 30, 2020
View reviewed changes
Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

LGTM by me. The mock pad reader may need to stay around to keep tests passing but I'm not sure.

shannonwells
shannonwells approved these changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

Well that was easy. I literally said this out loud.
:shipit:

@laser laser force-pushed the chores/consume-extracted-padreader branch from c32d45f to 75ccc89 Compare January 30, 2020 21:28
@laser
Copy link
Contributor Author

laser commented Jan 30, 2020

update: I've deleted the generated PadReader.go mock

@codecov-io
Copy link

Codecov Report

Merging #103 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #103   +/-   ##
=======================================
  Coverage   34.63%   34.63%           
=======================================
  Files          36       36           
  Lines        2712     2712           
=======================================
  Hits          939      939           
  Misses       1616     1616           
  Partials      157      157
Impacted Files Coverage Δ
storagemarket/impl/provider_states.go 0% <0%> (ø) ⬆️
pieceio/pieceio.go 84.38% <100%> (ø) ⬆️

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 cd7aa31...75ccc89. Read the comment docs.

@laser laser merged commit 505571b into master Jan 30, 2020
@laser
Copy link
Contributor Author

laser commented Jan 30, 2020

thnx

@laser laser deleted the chores/consume-extracted-padreader branch January 30, 2020 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ingar ingar ingar approved these changes

@hannahhoward hannahhoward hannahhoward approved these changes

@shannonwells shannonwells shannonwells approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@laser @codecov-io @ingar @shannonwells @hannahhoward