Skip to content

Conversation

@asurkov
Copy link
Contributor

@asurkov asurkov commented Nov 7, 2017

No description provided.

@asurkov asurkov requested a review from astansler November 7, 2017 22:16
@asurkov asurkov changed the base branch from master to develop November 7, 2017 22:16
}

getDiffsObservable(tail).blockingSubscribe { (commit, diffs) ->
getDiffsObservable(tail).subscribe( { (commit, diffs) ->
Copy link
Member

Choose a reason for hiding this comment

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

Asynchronous subscribe. Is it working not in a test mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean if it was called from Main, then yes, it seems so. The problem of blockingSubscribe is it collects all items before the items are delivered to a consumer, which is a bit slower than it could be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anatolykislov if you are skeptical about subscribe part, then let's back it out and deal with it separately in different issue?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's do it

// TODO(anatoly): CodeLongevity hash from observable.
try {
CodeLongevity(serverRepo, filteredEmails, git).updateStats(api)
CodeLongevity(serverRepo, filteredEmails, git, onError).
Copy link
Member

Choose a reason for hiding this comment

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

It's better to move point to next line :)

@asurkov asurkov merged commit 22f488b into develop Nov 17, 2017
@asurkov asurkov deleted the lgv_reporterror branch November 17, 2017 01:08
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.

3 participants