Skip to content
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

[iTunesLibrary] Update bindings for Xcode 13.0 beta 1 #12038

Merged
merged 10 commits into from
Jul 9, 2021

Conversation

rachelkang
Copy link
Contributor

No description provided.

@rachelkang rachelkang added the note-highlight Worth calling out specifically in release notes label Jun 29, 2021
@rachelkang rachelkang added this to the xcode13.0 milestone Jun 29, 2021
Comment on lines 334 to 335
[Export ("master")]
bool Master { [Bind ("isMaster")] get; }
Copy link
Member

Choose a reason for hiding this comment

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

I see that you've moved the Master declaration here from above - but don't move code around unless really necessary, because it shows up as "removed+added" in the diff, which makes the diff harder to review.

On a more general note, changes that don't really change anything also makes it harder to run "git blame" to figure out when/what something happened, because the first thing that will come up is the commit that moved the code, which is not what you're looking for.

Copy link
Member

Choose a reason for hiding this comment

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

To add to that, if we where working with a xcode13 branch, it would make my life harder when doing merges and back ports :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

understood! fixed :)

Copy link
Member

@mandel-macaque mandel-macaque left a comment

Choose a reason for hiding this comment

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

Approving once the master move is undone.

Copy link
Member

@rolfbjarne rolfbjarne left a comment

Choose a reason for hiding this comment

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

👍

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build] Tests passed on Build. ✅

Tests passed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

🎉 All 86 tests passed 🎉

Pipeline on Agent XAMBOT-1096.BigSur'
Merge 009f45d into a64b030

Copy link
Member

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

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

After these two nits

@@ -307,9 +307,15 @@ interface ITLibPlaylist
[Export ("name")]
string Name { get; }

[Introduced (PlatformName.MacOSX, 10, 6, message: "Use 'isPrimary' instead.")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[Introduced (PlatformName.MacOSX, 10, 6, message: "Use 'isPrimary' instead.")]
[Introduced (PlatformName.MacOSX, 10, 6, message: "Use 'Primary' instead.")]

@@ -307,9 +307,15 @@ interface ITLibPlaylist
[Export ("name")]
string Name { get; }

[Introduced (PlatformName.MacOSX, 10, 6, message: "Use 'isPrimary' instead.")]
[Deprecated (PlatformName.MacOSX, 12, 0, message: "Use 'isPrimary' instead.")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[Deprecated (PlatformName.MacOSX, 12, 0, message: "Use 'isPrimary' instead.")]
[Deprecated (PlatformName.MacOSX, 12, 0, message: "Use 'Primary' instead.")]

@dalexsoto
Copy link
Member

We use the .NET name in the message strings, this is the reason behind the change isPrimary to Primary

Comment on lines 304 to 305
[Introduced (PlatformName.MacOSX, 10, 6, message: "Use 'ITLibPlaylistPropertyPrimary' instead.")]
[Deprecated (PlatformName.MacOSX, 12, 0, message: "Use 'ITLibPlaylistPropertyPrimary' instead.")]
Copy link
Member

@dalexsoto dalexsoto Jun 30, 2021

Choose a reason for hiding this comment

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

This one is also not needed

Suggested change
[Introduced (PlatformName.MacOSX, 10, 6, message: "Use 'ITLibPlaylistPropertyPrimary' instead.")]
[Deprecated (PlatformName.MacOSX, 12, 0, message: "Use 'ITLibPlaylistPropertyPrimary' instead.")]
[Deprecated (PlatformName.MacOSX, 12, 0, message: "Use 'Primary' instead.")]

@rachelkang
Copy link
Contributor Author

intro and xtro are all green!

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build] Tests passed on Build. ✅

Tests passed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

🎉 All 86 tests passed 🎉

Pipeline on Agent XAMBOT-1094.BigSur'
Merge 9da7629 into 0bfb745

@tj-devel709
Copy link
Contributor

This one looks good to "Squash and merge"!

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 85 tests passed.

Failed tests

  • monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2622 Passed: 2489 Inconclusive: 35 Failed: 1 Ignored: 132)

Pipeline on Agent XAMBOT-1100.BigSur'
Merge 3112f6d into 3b6f059

@rachelkang
Copy link
Contributor Author

Test failure unrelated - https://github.com/xamarin/maccore/issues/2443

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build] Tests passed on Build. ✅

Tests passed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

🎉 All 86 tests passed 🎉

Pipeline on Agent XAMBOT-1098.BigSur'
Merge 24673ce into f963938

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build] Tests passed on Build. ✅

Tests passed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

🎉 All 86 tests passed 🎉

Pipeline on Agent XAMBOT-1096.BigSur'
Merge 7f6c94f into ac5c157

@tj-devel709 tj-devel709 merged commit 924c006 into xamarin:main Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
note-highlight Worth calling out specifically in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants