Skip to content

[WatchKit] Add WatchKit bindings for Xcode 11.2 - #7202

Merged
3 commits merged into
xcode11.2from
unknown repository
Oct 11, 2019
Merged

[WatchKit] Add WatchKit bindings for Xcode 11.2#7202
3 commits merged into
xcode11.2from
unknown repository

Conversation

@ghost

@ghost ghost commented Oct 9, 2019

Copy link
Copy Markdown

Add WatchKit bindings for Xcode 11.2

@ghost
ghost requested a review from spouliot as a code owner October 9, 2019 20:34
@monojenkins

Copy link
Copy Markdown
Contributor

Build failure
Build failed or was aborted

Comment thread src/watchkit.cs
White,
}

[Watch (6,1), NoiOS]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the NoiOS necessary?

@ghost ghost Oct 9, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, the parent interface also has properties/methods that support iOS. WkInterfaceUserTrackingMode only supports WatchOS.

https://developer.apple.com/documentation/watchkit/wkinterfacemap/usertrackingmode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think it's needed (but there's no harm in it).
WatchKit used to be available on iOS - but it's not anymore (and there are stubs to maintain binary compatibility).

Comment thread src/watchkit.cs Outdated
interface WKInterfaceAuthorizationAppleIdButton {

[Export ("initWithTarget:action:")]
[Deprecated (PlatformName.WatchOS, 6,1, message: "Use initWithStyle:target:action:")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we have to use the C# method names here instead of the ObjC selectors

@ghost ghost Oct 9, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yup, that's a a placeholder, not sure what the convention is for referring to a c# constructor that's replacing the existing constructor. cc: @spouliot

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we used .ctor(...) in some places but
Use "new WKInterfaceAuthorizationAppleIdButton (WKInterfaceVolumeControlOrigin,NSObject,Selector)' instead."
might be better

in doubt use grep on the /src/ directory ;-)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yup, that's a a placeholder,

Put a do-not-merge label if there are pending questions on your side. That way someone else won't merge it (e.g. if you're not online). You can remove the label once you have answers.

@monojenkins

Copy link
Copy Markdown
Contributor

Build success
Build succeeded
⚠️ Mono built from source
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
Test run succeeded

Comment thread src/watchkit.cs Outdated

[Watch (6,1)]
[Export ("setUserTrackingMode:animated:")]
void SetShowsUserLocation (WKInterfaceMapUserTrackingMode mode, bool animated);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wrong name -> should be SetUserTrackingMode

Comment thread src/watchkit.cs Outdated

[Watch (4,0)]
[Export ("enableWaterLock")]
[Deprecated (PlatformName.WatchOS, 6,1, message: "Use WKInterfaceDevice.enableWaterLock")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use C# (not ObjC) syntax + our format, e.g. Use 'X' instead. so

"Use 'WKInterfaceDevice.EnableWaterLock ()' instead.

Comment thread src/watchkit.cs Outdated

[Watch (6,1), NoiOS]
[Native]
enum WKInterfaceAuthorizationAppleIDButtonStyle: long {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Id, no ID since it's not an acronym

Comment thread src/watchkit.cs
White,
}

[Watch (6,1), NoiOS]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think it's needed (but there's no harm in it).
WatchKit used to be available on iOS - but it's not anymore (and there are stubs to maintain binary compatibility).

Comment thread src/watchkit.cs Outdated
interface WKInterfaceAuthorizationAppleIdButton {

[Export ("initWithTarget:action:")]
[Deprecated (PlatformName.WatchOS, 6,1, message: "Use initWithStyle:target:action:")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we used .ctor(...) in some places but
Use "new WKInterfaceAuthorizationAppleIdButton (WKInterfaceVolumeControlOrigin,NSObject,Selector)' instead."
might be better

in doubt use grep on the /src/ directory ;-)

Comment thread src/watchkit.cs Outdated
interface WKInterfaceAuthorizationAppleIdButton {

[Export ("initWithTarget:action:")]
[Deprecated (PlatformName.WatchOS, 6,1, message: "Use initWithStyle:target:action:")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yup, that's a a placeholder,

Put a do-not-merge label if there are pending questions on your side. That way someone else won't merge it (e.g. if you're not online). You can remove the label once you have answers.

@prmallic
prmallic self-requested a review October 10, 2019 20:51
@monojenkins

Copy link
Copy Markdown
Contributor

Build failure
Build succeeded
⚠️ Mono built from source
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 89 tests passed.

Failed tests

  • monotouch-test/iOS Unified 64-bits - simulator/Debug: Crashed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar): Crashed

@ghost
ghost merged commit b3d566d into dotnet:xcode11.2 Oct 11, 2019
mandel-macaque pushed a commit to mandel-macaque/xamarin-macios that referenced this pull request Oct 14, 2019
* [WatchKit] Add WatchKit bindings for Xcode 11.2
mandel-macaque pushed a commit to mandel-macaque/xamarin-macios that referenced this pull request Oct 15, 2019
* [WatchKit] Add WatchKit bindings for Xcode 11.2
@rolfbjarne rolfbjarne added the not-notes-worthy Ignore for release notes label Sep 10, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

not-notes-worthy Ignore for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants