- Personal preference but I like `[TV (13,0)]` better than `[TV (13, 0)]` (more concise).- As much as possible try to minize diff changes. For instance if there was a `[NoTV]` and now new availability attributes are needed, add them on a new line.  This was the original `[NoTV]` is unchanged, not critical in this case but we'd still be able to blame the original author, finally that reduces the amount of changes to review in the PR.- Fix some { that go on the same line for types (mono coding guideline).

Remove as many !MONOMAC as possible (+fixing test failure)

d625527

- Fix Mac introspection test (one inline .ctor was incorrectly marked as !MONOMAC)- Use and abuse of using statements to avoid !MONOMAC ifdefs at all costs.  These ifdefs are just painful and make the code hard to read (very error prone). It is much simpler to add [NoMac] everywhere and have the right using in place.

More style fixes and !MONOMAC removal

45dbcd3

Fix routePickerButtonColorForState API name

b9d3ba7

Remove weak delegate change for now (BREAKING CHANGE)

755822e

Type Changed: AVKit.AVRoutePickerViewModified properties:public ~virtual~ IAVRoutePickerViewDelegate Delegate { get; set; }

Revert "Remove weak delegate change for now (BREAKING CHANGE)"

957f4ee

This reverts commit755822e.

Just remembered about IsVirutal on Wrap, here's how it's not a breaki…

4f58b25

…ng change anymore

mandel-macaque approved these changes

Aug 26, 2019

View reviewed changes

Copy link

Contributor

VincentDondain commentedAug 26, 2019

build

VincentDondain requested review fromrolfbjarne andspouliot

August 26, 2019 22:58

VincentDondain approved these changes

Aug 26, 2019

View reviewed changes

Copy link

Collaborator

monojenkins commentedAug 27, 2019

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

spouliot suggested changes

Aug 27, 2019

View reviewed changes

src/avkit.cs Outdated


		[TV(13,0),NoiOS,NoWatch,NoMac]
		[Export("nextChannelInterstitialViewControllerForPlayerViewController:")]
		UIViewControllerNextChannelInterstitialViewController(AVPlayerViewControllerplayerViewController);

Copy link

Contributor

spouliotAug 27, 2019

Choose a reason for hiding this comment

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

lack of verb/action: prefix withGet

src/avkit.cs


		[iOS(12,0),NoTV,NoWatch,NoMac]
		[Export("playerViewController:willBeginFullScreenPresentationWithAnimationCoordinator:"),EventArgs("AVPlayerViewFullScreenPresentationWillBegin")]
		voidWillBeginFullScreenPresentation(AVPlayerViewControllerplayerViewController,IUIViewControllerTransitionCoordinatorcoordinator);

Copy link

Contributor

spouliotAug 27, 2019

Choose a reason for hiding this comment

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

please check/test if12,0 is correct (a bit tricker for delegates)

src/avkit.cs


		[iOS(12,0),NoTV,NoWatch,NoMac]
		[Export("playerViewController:willEndFullScreenPresentationWithAnimationCoordinator:"),EventArgs("AVPlayerViewFullScreenPresentationWillEnd")]
		voidWillEndFullScreenPresentation(AVPlayerViewControllerplayerViewController,IUIViewControllerTransitionCoordinatorcoordinator);

Copy link

Contributor

spouliotAug 27, 2019

Choose a reason for hiding this comment

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

same

src/avkit.cs Outdated


		[TV(13,0),NoiOS,NoWatch,NoMac]
		[Export("previousChannelInterstitialViewControllerForPlayerViewController:")]
		UIViewControllerPreviousChannelInterstitialViewController(AVPlayerViewControllerplayerViewController);

Copy link

Contributor

spouliotAug 27, 2019

Choose a reason for hiding this comment

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

prefix withGet

rolfbjarne approved these changes

Aug 27, 2019

View reviewed changes

Fixes based on Sebastien's comments

5b60b66

- Play it safe and don't use [iOS (12,0] availability. After all it was added in Xcode 11, sounds like it was private in Xcode 10. People can still use the API on earlier versions if they really need to and changing this later wouldn't be a breaking change.- Use verbs where we missed them.

Copy link

Contributor