Conversation
There was a problem hiding this comment.
what if we add more argument types, like int with proper integer parsing?
There was a problem hiding this comment.
I don't think it would make sense convoluting the package for that, it would be better for the command to just parse the int themselves.
There was a problem hiding this comment.
This function is simply out of control. There's so much going on. refactor. Think of composing simple units.
core/commands2/add.go
Outdated
There was a problem hiding this comment.
what if addDagnode returns an error!? please never ignore errors. :( (hm, i think it should be a compiler error to ignore func output.)
There was a problem hiding this comment.
Oops, didn't mean to do that. Fixed in 3be02a3
|
CR done. looots of comments up there o/ One major thing is tests. I worry there are subtle bugs in the File/Reader stuff. See: http://golang.org/src/pkg/mime/multipart/multipart_test.go for inspiration This is a short summary of the major things, though please address all
|
|
@mappum attention to #357 (comment) -- which is now a hidden-diff comment |
|
(The error Travis is complaining about came from bitswap BTW) |
|
|
This PR is ready for CR.
Overview of changes:
commands.Fileinterface that implementsio.Reader(which also has a filename and can be read in parts)Request#Arguments()to a separateRequest#Files()commands.Files)commands/http