-
Notifications
You must be signed in to change notification settings - Fork 518
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
[NSObject] Unify some code between iOS and macOS. #15302
[NSObject] Unify some code between iOS and macOS. #15302
Conversation
* Add these methods to shared code so they're available on all platforms (they're already available on mobile platforms), since there's no reason to exclude them on macOS: * NSObject.Init * NSObject.Alloc * NSObject.InvokeInBackground * Remove unused usings. * Move identical code in platform-specific files to shared code.
💻 [PR Build] Tests on macOS Mac Catalina (10.15) passed 💻✅ All tests on macOS Mac Catalina (10.15) passed. Pipeline on Agent |
❌ [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌Failed tests are:
Pipeline on Agent |
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes).NET (No breaking changes)✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)✅ Generator diffGenerator diff is empty Pipeline on Agent |
❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌Tests failed on VSTS: simulator tests iOS. Test results3 tests failed, 145 tests passed.Failed tests
Pipeline on Agent XAMBOT-1103.Monterey' |
Test failures are unrelated:
|
// using the parameterized Thread.Start to avoid capturing | ||
// the 'action' parameter (it'll needlessly create an extra | ||
// object). | ||
new System.Threading.Thread ((v) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole create a thread for one callback is worrying. This API might end up in mis-use more often than correct use.
new System.Threading.Thread ((v) => | |
new System.Threading.Thread (static (v) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's a somewhat useless and non-optimal API nowadays, but I think we should have the same API on all platforms.
Then we can deal with any problems with the API at the same time for all platforms (separately).
(they're already available on mobile platforms), since there's no reason to
exclude them on macOS: