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

Rename Event.failed to Event.error #60

Closed
inamiy opened this issue Oct 11, 2016 · 4 comments
Closed

Rename Event.failed to Event.error #60

inamiy opened this issue Oct 11, 2016 · 4 comments

Comments

@inamiy
Copy link
Contributor

inamiy commented Oct 11, 2016

This is just a matter of taste, but observer.sendFailed() has been renamed to observer.send(error:), so I think Event.failed should be renamed to Event.error as well (and also other methods like on(failed:)).

Or, renaming observer.send(error:) to observer.send(failed:) improves consistency too.
(I prefer send(error:) though)

@andersio
Copy link
Member

andersio commented Oct 11, 2016

The event was intentionally (re)named failed as being one of the terminating event. Though I agree we should take a second look at the consistency.

ReactiveCocoa/ReactiveCocoa#2505
ReactiveCocoa/ReactiveCocoa#2360
ReactiveCocoa/ReactiveCocoa#2349

@ikesyo
Copy link
Member

ikesyo commented Oct 11, 2016

And also #8.

@mdiep
Copy link
Contributor

mdiep commented Oct 11, 2016

I think it makes sense as-is. failed is the event; error is the instance that initiates/describes it. An error causes the stream to fail.

@inamiy
Copy link
Contributor Author

inamiy commented Oct 12, 2016

Thanks for references.
I still find the naming inconsistency in Observer.swift#L78-L104 ("send" should be followed by event name), but it might be an exception for argument label case.

Anyway, this is too trivial issue to discuss, so I will close.

@inamiy inamiy closed this as completed Oct 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants