Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[corelocation] Update for Xcode 11 beta 1 to 4#6600

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
spouliot merged 4 commits intodotnet:xcode11fromspouliot:xcode11-corelocation-b1-3
Jul 18, 2019

Conversation

@spouliot
Copy link
Contributor

No description provided.

rolfbjarne reacted with thumbs up emoji
@monojenkins
Copy link
Collaborator

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

Test results

9 tests failed, 82 tests passed.

Failed tests

  • xammac tests/Mac Modern/Debug: BuildFailure
  • xammac tests/Mac Modern/Release: BuildFailure
  • xammac tests/Mac Modern/Release: BuildFailure
  • monotouch-test/tvOS - simulator/Debug: BuildFailure
  • monotouch-test/tvOS - simulator/Debug (static registrar): BuildFailure
  • monotouch-test/tvOS - simulator/Release (all optimizations): BuildFailure
  • monotouch-test/watchOS 32-bits - simulator/Debug: BuildFailure
  • monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): BuildFailure
  • monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): BuildFailure

[NoWatch][NoTV]
[iOS(6,0)]
[Mac(10,9)]
[Deprecated(PlatformName.iOS,13,0,message:"Not used anymore. It will always return .false'.")]
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering if ".false" is a typo?

dalexsoto reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

yes it should have been'false'
good catch !

@monojenkins
Copy link
Collaborator

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

@mandel-macaque
Copy link
Contributor

We probably want to double check with the new Xcode 11 beta 4.

[NoWatch,NoTV,NoMac,iOS(13,0)]
[Export("locationManager:didRangeBeacons:satisfyingConstraint:")]
[EventArgs("CLRegionBeaconsConstraintRanged")]
voidDidRangeBeaconsSatisfyingConstraint(CLLocationManagermanager,CLBeacon[]beacons,CLBeaconIdentityConstraintbeaconConstraint);
Copy link
Contributor

Choose a reason for hiding this comment

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

CanDidRangeBeaconsSatisfyingConstraint be shortened toDidRangeBeacons - since the params are different it won't cause a namespace issue, right?

dalexsoto reacted with thumbs up emoji
Copy link
ContributorAuthor

@spouliotspouliotJul 18, 2019
edited
Loading

Choose a reason for hiding this comment

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

Good catch! It was my first attempt too - but sadly it can't :(

The problem are that:

  • the name is already used in the type (even if overloads are fine most of the time);
  • a*Delegate type;
  • it uses[EventArgs];

so this cause a conflict in the generated code.

I don't have this PR built right now - but I'll update the comment with the code (easier to see what's happening this way).

Update

So the[EventArgs] attribute is used in conjunction with theEvents property of[BaseType]. This allows us to expose C# events as a better alternative (there are gotchas) to implementing a*Delegate.

[BaseType(typeof(NSObject),Delegates=newstring[]{"WeakDelegate"},Events=newType[]{typeof(CLLocationManagerDelegate)})]partialinterfaceCLLocationManager{

But the extra events means a conflict because we already have aDidRangeBeacons method inCLLocationManagerDelegate, so the generator already created an event using that name:

publiceventEventHandler<CLRegionBeaconsRangedEventArgs> DidRangeBeacons{add{ EnsureCLLocationManagerDelegate().didRangeBeacons+= value;}remove{EnsureCLLocationManagerDelegate().didRangeBeacons-=value;}}

and we can't overload events in C# (even if the args are different).

So to avoid the compiler error a new, different name must be used - and I chooseDidRangeBeaconsSatisfyingConstraint

dalexsoto and whitneyschmidt reacted with thumbs up emoji
NSUuidUuid{get;}

[NullAllowed,Export("major",ArgumentSemantic.Copy)]
[BindAs(typeof(short?))]
Copy link
Contributor

Choose a reason for hiding this comment

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

When do we useBindAs?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

There's a few cases, the most common one isNSNumber, like above. Another one are to support "smart enums" (using a C# enum that maps to severalNSString constants).

So for some API Apple prefers to use a reference type, likeNSNumber, over a more specific value type. Why ? mainly because they want a nullable properties

But

NSNumberMajor{get;}

does not tell the API consumer if it should except ashort (like it's meant to), afloat or even abool. That does not create a very discoverable API and we can do a lot better in C# with nullable? types :)

Why not

short?Major{get;}

Because our runtime must know this anNSNumber natively - e.g. it's needed to allow us to override the property in a subclass and have some ObjC code call us. So the declaration remains anNSNumber but we give the generator a hint to add extra (conversion) code to expose a nicer API to our developers.

whitneyschmidt reacted with thumbs up emoji
Copy link
Member

Choose a reason for hiding this comment

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

whitneyschmidt reacted with heart emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you both for the detailed explanation! 👍

@spouliot
Copy link
ContributorAuthor

@mandel-macaque there's no change in corelocation for beta 4 (but I'll fix the typo found by@tj-devel709)

@spouliotspouliot changed the title[corelocation] Update for Xcode 11 beta 1 to 3[corelocation] Update for Xcode 11 beta 1 to 4Jul 18, 2019
@spouliotspouliot added this to thexcode11 milestoneJul 18, 2019
@monojenkins
Copy link
Collaborator

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

@spouliotspouliot merged commit3648704 intodotnet:xcode11Jul 18, 2019
@spouliotspouliot deleted the xcode11-corelocation-b1-3 branchJuly 18, 2019 18:16
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@dalexsotodalexsotodalexsoto approved these changes

@tj-devel709tj-devel709tj-devel709 left review comments

+2 more reviewers

@whitneyschmidtwhitneyschmidtwhitneyschmidt approved these changes

@mandel-macaquemandel-macaquemandel-macaque approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

xcode11

Development

Successfully merging this pull request may close these issues.

6 participants

@spouliot@monojenkins@mandel-macaque@dalexsoto@tj-devel709@whitneyschmidt

[8]ページ先頭

©2009-2025 Movatter.jp