[StoreKit] Update bindings for Xcode 11.4 beta 3 #8104

whitneyschmidt · 2020-03-12T20:30:02Z

Original PR against xcode11.4 branch can be found here: #8081

mandel-macaque

Some minor details.

mandel-macaque · 2020-03-12T20:35:50Z

src/storekit.cs

@@ -76,21 +82,23 @@ partial interface SKDownload {
 		void DeleteContentForProduct (string productId);
 #endif

-		[Mac (10,14)]
+		[Mac (10, 14)]


Not needed space change.

mandel-macaque · 2020-03-12T20:35:54Z

src/storekit.cs

 		[Field ("SKDownloadTimeRemainingUnknown")]
 		double TimeRemainingUnknown { get; }

-		[Mac (10,11)]
+		[Mac (10, 11)]


Not needed space change.

mandel-macaque · 2020-03-12T20:37:47Z

src/storekit.cs

 		[NullAllowed] // by default this property is null
 		[Export ("applicationUsername", ArgumentSemantic.Copy)][New]
 		string ApplicationUsername { get; set; }

-		[iOS (8,3), Mac (10,14)]
+		[Watch (6, 2), iOS (8,3), Mac (10,14)]


We should be consistent between [Watch (6, 2)] and [Watch (6,2)]. I know we are not very, but in this case you are using both in the same PR.

mandel-macaque · 2020-03-12T20:40:08Z

src/frameworks.sources

@@ -2029,6 +2029,7 @@ WATCHOS_FRAMEWORKS =        \
 	PassKit                 \
 	PushKit			\
 	Security                \
+	StoreKit                \


Looking at the complete file, StoreKit is present in all platforms, Mac, iOS, tvOS and watchOS. At this point will should remove it f rom all the subsets and add it to COMMON_FRAMEWORKS

monojenkins · 2020-03-12T21:11:06Z

Build failure
❌ Build failed or was aborted

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

spouliot

please double check that every public type either

has a [Watch(6,2)] attribute
has a [NoWatch] attribute, or
is under a #if !WATCH block

spouliot · 2020-03-12T21:22:33Z

src/storekit.cs

@@ -132,34 +140,39 @@ interface SKMutablePayment {
 		[Availability (Deprecated = Platform.iOS_5_0, Message = "Use 'PaymentWithProduct (SKProduct)' after fetching the list of available products from 'SKProductRequest' instead.")]
 		SKMutablePayment PaymentWithProduct (string identifier);

+		[Watch (6, 2)]


those should be on the type
because without a [Watch] on the type we'll assume it's been there since watchOS 2.0 (and that's not correct)

Ah, got it. I will go through all the APIs and mark them as appropriate :)

spouliot · 2020-03-12T21:22:56Z

src/storekit.cs

@@ -24,6 +27,7 @@ partial interface SKDownload {

 		[iOS (12,0)]
 		[TV (12,0)]
+		[Watch (6, 2)]


move to type (see other message)

monojenkins · 2020-03-12T23:52:27Z

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

3 tests failed, 91 tests passed.

Failed tests

Xtro/Mac: Failed (Test run failed.)
introspection/watchOS 32-bits - simulator/Debug: Failed
introspection/watchOS 32-bits - simulator/Debug (watchOS 3.2): Failed

mandel-macaque · 2020-03-13T16:40:17Z

src/storekit.cs

@@ -393,12 +419,14 @@ interface SKRequestDelegate {
 	[Mac (10,9)]
 	[BaseType (typeof (SKRequest))]


As @spouliot mentioned with others, move the [Watch (6,2)] to the top, just need to add one and the wraps should not need any attr decoration.

mandel-macaque · 2020-03-13T21:44:16Z

@whitneyschmidt looks like there is a introspection issue with SKProductStorePromotionController, looking at the docs, is not present in the watchos

whitneyschmidt · 2020-03-13T22:29:49Z

src/storekit.cs

@@ -27,18 +31,22 @@ partial interface SKDownload {
 		[Export ("state")]
 		SKDownloadState State { get; }
 #if MONOMAC
+		[NoWatch] // not necessary b/c #if MONOMAC, so this will not even compile for ios/watch/tvos?


It seems like this is not necessary because we have #if MONOMAC and it was not marked as [NoTV][NoiOS] before?

no, it's not required
it would tell, in Xamarin.Mac.dll, that this is not available on watchOS - but that's not very useful

whitneyschmidt · 2020-03-13T22:30:53Z

src/storekit.cs

 		[Deprecated (PlatformName.MacOSX, 10,15, message: "Use 'IsDownloadable' instead.")]
 		[Export ("downloadable")]
 		bool Downloadable { get; }
 #elif !XAMCORE_4_0
+		[NoWatch] // IS THIS RIGHT?


Not sure about this one, but since it is Obsolete it seems like we would not want users to have access to it starting in [Watch (6, 2)]?

no, it's not needed

some times when Apple enable an existing framework in a new platform then bring everything, including deprecated API :(
Since this is replaced by something newer/better, that is always available on the new platform, then we can reduce warnings and let intellisense provide better choice to the developers

if xtro complains (because it's in the header) then add a line in watchOS-StoreKit.ignore

monojenkins · 2020-03-13T23:56:57Z

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

1 tests failed, 93 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed

spouliot · 2020-03-14T02:50:02Z

src/storekit.cs

@@ -6,7 +6,7 @@
 //   Miguel de Icaza
 //
 // Copyright 2009, Novell, Inc.
-// Copyright 2012 Xamarin Inc.
+// Copyright 2020 Xamarin Inc.


don't remove, replace - just append
in this case it should be a new line for 2020 / Microsoft

spouliot · 2020-03-14T02:51:32Z

src/storekit.cs

@@ -27,18 +31,22 @@ partial interface SKDownload {
 		[Export ("state")]
 		SKDownloadState State { get; }
 #if MONOMAC
+		[NoWatch] // not necessary b/c #if MONOMAC, so this will not even compile for ios/watch/tvos?


no, it's not required
it would tell, in Xamarin.Mac.dll, that this is not available on watchOS - but that's not very useful

spouliot · 2020-03-14T02:54:42Z

src/storekit.cs

 		[Deprecated (PlatformName.MacOSX, 10,15, message: "Use 'IsDownloadable' instead.")]
 		[Export ("downloadable")]
 		bool Downloadable { get; }
 #elif !XAMCORE_4_0
+		[NoWatch] // IS THIS RIGHT?


no, it's not needed

some times when Apple enable an existing framework in a new platform then bring everything, including deprecated API :(
Since this is replaced by something newer/better, that is always available on the new platform, then we can reduce warnings and let intellisense provide better choice to the developers

if xtro complains (because it's in the header) then add a line in watchOS-StoreKit.ignore

mandel-macaque

Approving taking into account that @spouliot are taken care of.

dalexsoto

LGTM 👍 after Sebastien's comments are addressed

monojenkins · 2020-03-16T17:10:50Z

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

1 tests failed, 93 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed

whitneyschmidt · 2020-03-16T17:51:54Z

build

spouliot · 2020-03-16T19:51:16Z

@whitneyschmidt please don't issue a new build without checking the logs first. This was a known HE0038 issue so it could be merged (with a note).

monojenkins · 2020-03-16T20:32:07Z

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

whitneyschmidt added 3 commits March 12, 2020 16:25

initial commit of StoreKit changes

b3e4791

frameworks.cs to 6,2

b5616de

respond to round 1 feedback

7e6196a

whitneyschmidt requested review from spouliot and mandel-macaque March 12, 2020 20:30

whitneyschmidt requested review from chamons and rolfbjarne as code owners March 12, 2020 20:30

whitneyschmidt requested a review from dalexsoto March 12, 2020 20:30

mandel-macaque requested changes Mar 12, 2020

View reviewed changes

whitneyschmidt added 2 commits March 12, 2020 16:55

move StoreKit into COMMON_FRAMEWORKS

01f866f

fix manuel formatting suggestions

e7719ae

spouliot suggested changes Mar 12, 2020

View reviewed changes

rolfbjarne approved these changes Mar 13, 2020

View reviewed changes

mandel-macaque requested changes Mar 13, 2020

View reviewed changes

add availability attributes to everything, add comments with questions

2ab6f4a

whitneyschmidt commented Mar 13, 2020

View reviewed changes

spouliot suggested changes Mar 14, 2020

View reviewed changes

mandel-macaque approved these changes Mar 14, 2020

View reviewed changes

rolfbjarne approved these changes Mar 16, 2020

View reviewed changes

dalexsoto approved these changes Mar 16, 2020

View reviewed changes

remove extra [NoWatch], fix copyright

b7538f9

spouliot approved these changes Mar 16, 2020

View reviewed changes

whitneyschmidt merged commit e085125 into xamarin:d16-5-xcode11.4 Mar 16, 2020

whitneyschmidt added the notes-mention Deserves a mention in release notes label Sep 9, 2020

whitneyschmidt added this to the xcode11 milestone Sep 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[StoreKit] Update bindings for Xcode 11.4 beta 3 #8104

[StoreKit] Update bindings for Xcode 11.4 beta 3 #8104

whitneyschmidt commented Mar 12, 2020

mandel-macaque left a comment

mandel-macaque Mar 12, 2020

mandel-macaque Mar 12, 2020

mandel-macaque Mar 12, 2020

mandel-macaque Mar 12, 2020

monojenkins commented Mar 12, 2020

spouliot left a comment

spouliot Mar 12, 2020

whitneyschmidt Mar 12, 2020

spouliot Mar 12, 2020

monojenkins commented Mar 12, 2020

Failed tests

mandel-macaque Mar 13, 2020

mandel-macaque commented Mar 13, 2020

whitneyschmidt Mar 13, 2020

spouliot Mar 14, 2020

whitneyschmidt Mar 13, 2020

spouliot Mar 14, 2020

monojenkins commented Mar 13, 2020

Failed tests

spouliot Mar 14, 2020

spouliot Mar 14, 2020

spouliot Mar 14, 2020

mandel-macaque left a comment

dalexsoto left a comment

monojenkins commented Mar 16, 2020

Failed tests

whitneyschmidt commented Mar 16, 2020

spouliot commented Mar 16, 2020

monojenkins commented Mar 16, 2020

		@@ -393,12 +419,14 @@ interface SKRequestDelegate {
		[Mac (10,9)]
		[BaseType (typeof (SKRequest))]

[StoreKit] Update bindings for Xcode 11.4 beta 3 #8104

[StoreKit] Update bindings for Xcode 11.4 beta 3 #8104

Conversation

whitneyschmidt commented Mar 12, 2020

mandel-macaque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Mar 12, 2020

spouliot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Mar 12, 2020

Test results

Failed tests

Choose a reason for hiding this comment

mandel-macaque commented Mar 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Mar 13, 2020

Test results

Failed tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mandel-macaque left a comment

Choose a reason for hiding this comment

dalexsoto left a comment

Choose a reason for hiding this comment

monojenkins commented Mar 16, 2020

Test results

Failed tests

whitneyschmidt commented Mar 16, 2020

spouliot commented Mar 16, 2020

monojenkins commented Mar 16, 2020