Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

make swarm connect return an error when it fails #1900

Merged
merged 1 commit into from
Oct 28, 2015
Merged

Conversation

whyrusleeping
Copy link
Member

@diasdavid made me look at this code a little more and i realized that if it fails, you wont know about it through the api.

This should fix that.

License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
@daviddias
Copy link
Member

sweet :D thanks @whyrusleeping !

@RichardLitt RichardLitt mentioned this pull request Oct 26, 2015
35 tasks
@whyrusleeping whyrusleeping added RFCR and removed status/in-progress In progress labels Oct 27, 2015
jbenet added a commit that referenced this pull request Oct 28, 2015
make swarm connect return an error when it fails
@jbenet jbenet merged commit 62cb6eb into master Oct 28, 2015
@jbenet jbenet deleted the fix/swarm-con-err branch October 28, 2015 00:04
@jbenet
Copy link
Member

jbenet commented Oct 28, 2015

👍

@jbenet
Copy link
Member

jbenet commented Oct 28, 2015

@whyrusleeping i merged that too aggressively, i think we want to add a test case here to ensure that a failure does indeed fail with an API error.

@jbenet
Copy link
Member

jbenet commented Oct 28, 2015

@diasdavid @whyrusleeping how do you guys feel about maybe leading feature development with test cases, i.e. this report could've started with a test case.

@whyrusleeping
Copy link
Member Author

how do you guys feel about maybe leading feature development with test cases, i.e. this report could've started with a test case.

Not opposed to that.

@daviddias
Copy link
Member

I like that. In this case, the test was on the node-ipfs-api, should that be considered as a valid test case?

@jbenet
Copy link
Member

jbenet commented Oct 28, 2015

@diasdavid not easy to run test case, and too broad. I think it could be a small sharness test here. 


Sent from Mailbox

On Tue, Oct 27, 2015 at 8:20 PM, David Dias notifications@github.com
wrote:

I like that. In this case, the test was on the node-ipfs-api, should that be considered as a valid test case?

Reply to this email directly or view it on GitHub:
#1900 (comment)

@jbenet
Copy link
Member

jbenet commented Oct 28, 2015

Should be something that fails this repo CI I think


Sent from Mailbox

On Tue, Oct 27, 2015 at 8:20 PM, David Dias notifications@github.com
wrote:

I like that. In this case, the test was on the node-ipfs-api, should that be considered as a valid test case?

Reply to this email directly or view it on GitHub:
#1900 (comment)

@whyrusleeping whyrusleeping mentioned this pull request Nov 2, 2015
47 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants