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

Allow work factories with other return types than Observable #125

Merged
merged 4 commits into from
Nov 20, 2017

Conversation

mosamer
Copy link
Contributor

@mosamer mosamer commented Nov 12, 2017

This PR closes #123

  • Adds convenience initialiser that takes a work factory returning ObservableConvertibleType. This allows constructing Actions with work factory returning;
    • PrimitiveSequence such as Single, Completable, etc.
    • SharedSequence such as Driver
  • Adds type alias CompletableAction.
    • An action with a work factory returning a Completable
    • Its elements will emit no next events

@ashfurrow
Copy link
Member

Hey! Thanks for the PR, I'll try to take a look this afternoon.

Copy link
Member

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

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

Looks great! Good test coverage. Can you add a note to the changelog under "current master"? Thanks!

@@ -4,6 +4,8 @@ import RxCocoa

/// Typealias for compatibility with UIButton's rx.action property.
public typealias CocoaAction = Action<Void, Void>
/// Typealias for actions with work factory returns `Completable`
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a . at the end for consistency?

@mosamer mosamer force-pushed the primitive-sequence-init branch from 65bf3bf to f90e164 Compare November 17, 2017 10:42
@mosamer
Copy link
Contributor Author

mosamer commented Nov 17, 2017

I applied the required updates. As a side note, I see that CI was never triggered!

@ashfurrow
Copy link
Member

Oops! We recently changed CI providers and I hadn't yet configured Circle to build on PRs from forks. It should work going forward. Thanks for pointing that out!

Copy link
Member

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

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

Excellent work, thank you! Let me know if you'd like to see this released as a point release.

@ashfurrow ashfurrow merged commit 112cf45 into RxSwiftCommunity:master Nov 20, 2017
@mosamer mosamer deleted the primitive-sequence-init branch November 20, 2017 21:32
@mosamer
Copy link
Contributor Author

mosamer commented Nov 20, 2017

@ashfurrow A point release will be super! should I update pod spec? or will you take care of that?

@ashfurrow
Copy link
Member

No worries – I've got it pushed as 3.5.0, let me know if there are any issues 👍

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

Successfully merging this pull request may close these issues.

Allow init Action with PrimitiveSequence
2 participants