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

Stable bits from Pull Request #2634 #2792

Merged
merged 2 commits into from
Jun 2, 2016

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Jun 1, 2016

Stable bits from Pull Request #2634.

License: MIT
Signed-off-by: Kevin Atkinson <[email protected]>
@kevina
Copy link
Contributor Author

kevina commented Jun 1, 2016

I think continuous-integration/travis-ci/pr was getting confused as I committed two commits shortly after one another and it was attempting to build with the wrong commit id. Commit dbabcf9 does not touch coreunix/add.go.

@kevina
Copy link
Contributor Author

kevina commented Jun 1, 2016

@whyrusleeping this is ready for you to have a look, and okay with me if it gets merged.

@@ -54,7 +54,8 @@ func TestAddGCLive(t *testing.T) {

errs := make(chan error)
out := make(chan interface{})
adder, err := NewAdder(context.Background(), node, out)
adder, err := NewAdder(context.Background(), node.Pinning, node.Blockstore, node.DAG)
adder.Out = out
Copy link
Member

Choose a reason for hiding this comment

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

just in case (even though its a test), i'd put the adder.Out = out after the error check. If this call does error for some reason, its going to return a nil adder and we will panic on this line and lose information about what the real error was

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Your right fixed.

@whyrusleeping
Copy link
Member

One nitpick then LGTM

@whyrusleeping whyrusleeping added the need/author-input Needs input from the original author label Jun 2, 2016
This will make it easier to set up a specialized data pipeline.

License: MIT
Signed-off-by: Kevin Atkinson <[email protected]>
@kevina
Copy link
Contributor Author

kevina commented Jun 2, 2016

@whyrusleeping,okay I fixed the nitpick and all tests are passing.

@whyrusleeping
Copy link
Member

sweet, LGTM

@whyrusleeping whyrusleeping merged commit 082393d into ipfs:master Jun 2, 2016
@kevina kevina deleted the kevina/filestore-bits branch June 2, 2016 06:51
@@ -20,47 +20,47 @@ type Block interface {
}

// Block is a singular block of data in ipfs
type RawBlock struct {
type BasicBlock struct {
Copy link
Member

Choose a reason for hiding this comment

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

Curious why the name change?

Copy link
Contributor Author

@kevina kevina Aug 26, 2016

Choose a reason for hiding this comment

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

Note, that I originally named this RawBlock in commit b84cbec. Both commits where factored out of #2634/#2600. The name RawBlock was only a temporary name, I meant to squash this in b84cbec but messed up the rebase and didn't notice until after it was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/author-input Needs input from the original author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants