refactor(cmds): do not return errors embedded in result type#10527
refactor(cmds): do not return errors embedded in result type#10527
Conversation
| // } | ||
| // fmt.Println("Dir name:", dirEnt.Name) | ||
| // } | ||
| func LsIter(ctx context.Context, api UnixfsAPI, p path.Path, opts ...options.UnixfsLsOption) iter.Seq2[DirEntry, error] { |
There was a problem hiding this comment.
This is implemented to operate on any UnixfsAPI since their Ls should all work the same. Is this a good assumption / implementation, or should each UnixfsAPI provide its own LsIter?
There was a problem hiding this comment.
I guess it doesn't hurt, but it not used?
|
LGTM although improvement is marginal if this was already working code, with the side effect of altering CoreAPI (not sure if anyone builds anything on top of it other than Kubo itself). I personally don't like the new pattern much more than the old one. Something like
For me, the best pattern would be APIs like The implementation of Anyways this is just passing work around our own internal code, so it's all mostly the same. |
|
Thank you for the review. This PR was a much as a exploration to find a more preferable pattern as to satisfy the original issue. I really agree with
and also, let the caller decide whether or not the function should be called asynchronously. The one benefit this PR does have is not having to wait on both ctx and err chan in case err chan is not read, but that is not really important with regart to the above statement. I will take another pass over this to incorporate your feedback. |
44d91ab to
89890b4
Compare
0c8c900 to
22f399d
Compare
hsanjuan
left a comment
There was a problem hiding this comment.
I think everything has become much nicer to read.
| // } | ||
| // fmt.Println("Dir name:", dirEnt.Name) | ||
| // } | ||
| func LsIter(ctx context.Context, api UnixfsAPI, p path.Path, opts ...options.UnixfsLsOption) iter.Seq2[DirEntry, error] { |
There was a problem hiding this comment.
I guess it doesn't hurt, but it not used?
Asynchronous functions should return errors over channels instead of embedding the error in the result type. Closes #9974
5a0f36b to
2ae8495
Compare
Asynchronous functions should return errors over channels instead of embedding the error in the result type.
Closes #9974