-
Notifications
You must be signed in to change notification settings - Fork 507
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
[UIKit] Remove the preprocessor directives for watchOS. #15601
[UIKit] Remove the preprocessor directives for watchOS. #15601
Conversation
This change should have 0/null/nine/nada changes in the API. The goal is to remove the preprocessor directives to ensure that the xcode14 bindings have a smaller diff and are easier to review.
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.
Pointed out some parts I just wanted to double check
@@ -17413,7 +17511,9 @@ interface UIPrintInteractionControllerDelegate { | |||
UIPrinterCutterBehavior ChooseCutterBehavior (UIPrintInteractionController printInteractionController, NSNumber [] availableBehaviors); | |||
} | |||
|
|||
[NoTV] |
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.
Was this intentional?
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 diff is tricking you :) It was not removed, the delegate was added and then NoWatch was added to the attr. It is in the line 17516 now
@@ -18124,7 +18221,6 @@ partial interface UIMutableUserNotificationCategory { | |||
} | |||
|
|||
[Deprecated (PlatformName.iOS, 10, 0, message: "Use 'UserNotifications.UNNotificationAction' instead.")] | |||
[NoWatch] |
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.
Why the removal?
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.
Because it was a partial class in another part of the same file. I merged them to be in a single place.
@@ -18159,24 +18255,21 @@ partial interface UIUserNotificationAction : NSCopying, NSMutableCopying, NSSecu | |||
[Field ("UIUserNotificationTextInputActionButtonTitleKey")] | |||
NSString TextInputActionButtonTitleKey { get; } | |||
|
|||
#if !WATCH |
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.
Is this intentional?
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.
yes, if you look at it, I moved the selector from a partial interface to here. The issue is that I wanted to keep the original version yet keep the interface declaration in a single place.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Looks good to me, I have some mixed feelings about withespace/aligment but it is a good opportunity to fix them here so 👍
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
LGTM, though the CI should detect if this has an API change.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…/xamarin-macios into remove-preprocessor-uikit
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
🔥 [PR Build] Build failed 🔥Build failed for the job 'Detect API changes' Pipeline on Agent |
🔥 Unable to find the contents for the comment: D:\a\1\s\change-detection\results\gh-comment.md does not exist :fire Pipeline on Agent |
💻 [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 |
🔥 [CI Build] Test results 🔥Test results❌ Tests failed on VSTS: simulator tests 🎉 All 211 tests passed 🎉 Failures❌ mononative tests🔥 Failed catastrophically on VSTS: simulator tests - mononative (no summary found). Html Report (VSDrops) Download Successes✅ bcl: All 69 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
/sudo backport xcode14 |
Backport Job to branch xcode14 Created! The magic is happening here |
Hooray! Backport succeeded! Please see https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=6526007 for more details. |
This change should have 0/null/nine/nada changes in the API. The goal is
to remove the preprocessor directives to ensure that the xcode14
bindings have a smaller diff and are easier to review.