Skip to content

Conversation

@stefanomondino
Copy link
Member

This also updates dependencies to latest versions.

@ashfurrow
Copy link
Member

This is great! Thanks for the PR and detailed issue. @ishkawa when you have a chance, could you look this over? You're most familiar with that part of the code.

@ishkawa
Copy link
Member

ishkawa commented Jan 3, 2017

Thank you for the PR.
I'll review this today.

.of(executionStart.map { _ in true }, executionEnd.map { _ in false })
.merge()
.shareReplay(1)
.startWith(false)
Copy link
Member

Choose a reason for hiding this comment

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

I think shareReplay(1) should be placed after startWith(false) so that late subscribers can received the latest value as an initial value, not fixed false.

executing = Observable
    .of(executionStart.map { _ in true }, executionEnd.map { _ in false })
    .merge()
    .startWith(false)
    .shareReplay(1)

@ishkawa
Copy link
Member

ishkawa commented Jan 3, 2017

Could you run carthage bootstrap --use-submodules --no-build and commit submodule updates? CI job gets dependencies via git submodule update --init, not via carthage bootstrap, so we have to synchronize Carthage.resolved and .gitmodules.

Expected git status output look like below:

modified:   Carthage/Checkouts/Nimble
modified:   Carthage/Checkouts/Quick
modified:   Carthage/Checkouts/RxSwift

@fpillet
Copy link
Member

fpillet commented Jan 3, 2017

This PR really should be split in two. One part touches test frameworks, the other is unrelated and touches code

@fpillet
Copy link
Member

fpillet commented Jan 3, 2017

@stefanomondino would you mind closing this PR and opening another one with just the test frameworks / carthage module updates ? Thanks!

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.

4 participants