Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Oct 23, 2019

Add TVMLKit binding for xcode11.2b2

Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

That's a breaking change, all further members gets re-numbered.

You need to file an issue with Apple and (if we're lucky) they'll fix it properly...
Otherwise we'll need to document it (and like add an [Advice])

[ErrorDomain ("TVMLKitErrorDomain")]
public enum TVMLKitError : long {
Unknown = 1,
InvalidArguments,
Copy link
Contributor

Choose a reason for hiding this comment

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

The order of the enums matter. This changes makes InternetUnavailable to be the old one +1. Which is bad it the users do depend on the casting of enums to long/

Copy link
Contributor

Choose a reason for hiding this comment

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

It's basically an ABI break.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not just enum to long. It breaks interop too.

@ghost
Copy link
Author

ghost commented Oct 23, 2019

maccore issue: https://github.com/xamarin/maccore/issues/2039

@spouliot spouliot added the do-not-merge Do not merge this pull request label Oct 23, 2019
@spouliot spouliot added this to the xcode11.2 milestone Oct 23, 2019
Copy link
Contributor

@VincentDondain VincentDondain left a comment

Choose a reason for hiding this comment

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

Not sure why Sebastien added do-not-merge shouldn't we close this PR?

@monojenkins
Copy link
Collaborator

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

Test results

2 tests failed, 89 tests passed.

Failed tests

  • introspection/iOS Unified 64-bits - simulator/Debug: Failed
  • introspection/tvOS - simulator/Debug: Failed

@ghost ghost closed this Oct 24, 2019
@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

do-not-merge Do not merge this pull request not-notes-worthy Ignore for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants