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

Fixed DelegateProxy to check if it's running on main thread #1882

Merged
merged 7 commits into from Mar 9, 2019
Merged

Fixed DelegateProxy to check if it's running on main thread #1882

merged 7 commits into from Mar 9, 2019

Conversation

purple-x
Copy link
Contributor

Addressing issue: #1874

RxSwift/Schedulers/MainScheduler.swift Outdated Show resolved Hide resolved
Platform/DispatchQueue+Extensions.swift Outdated Show resolved Hide resolved
@RxPullRequestBot
Copy link

1 Warning
⚠️ No CHANGELOG changes made

Generated by 🚫 Danger

@kzaher
Copy link
Member

kzaher commented Mar 8, 2019

Hi @ryu1006 ,

could you please take a look at Linux unit tests. I would like to release this with a patch release this weekend.

Otherwise looks ok to me.

@freak4pc Could you please smoke test this in a real app. I don't currently have access to any bigger codebase.

@freak4pc
Copy link
Member

freak4pc commented Mar 8, 2019

@kzaher I'll give it a whirl and let you know.

@purple-x
Copy link
Contributor Author

purple-x commented Mar 8, 2019

@kzaher checked and saw that isMainThread is not implemented in Linux's Foundation. Fortunately enough, majority of the instances for ensureRunningOnMainThread is only on files that ensure it's not running on Linux or is in Debug. Only VirtualTimeScheduler does not have these checks, so reverted it to the previous ensureExecutingOnScheduler function.

@freak4pc
Copy link
Member

freak4pc commented Mar 8, 2019

@kzaher I've run ~500 unit tests of a larger codebase and everything seems to run smoothly.
I guess the main pieces we have counting on DelegateProxy is RxDataSources - and that seems to work and pass tests with no issues.

@kzaher kzaher merged commit 2ae13c4 into ReactiveX:develop Mar 9, 2019
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.

None yet

5 participants