NotificationsYou must be signed in to change notification settings
Fork548
Star2.8k

[UIKit] Partial update to Xcode 11 Beta 1 and 2 - Part 1 of ?#6392

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

Jump to bottom

Merged

dalexsoto merged 9 commits intodotnet:xcode11fromdalexsoto:alex-xcode11-uikit

Jun 21, 2019

Merged

[UIKit] Partial update to Xcode 11 Beta 1 and 2 - Part 1 of ?#6392

dalexsoto merged 9 commits intodotnet:xcode11fromdalexsoto:alex-xcode11-uikit

Jun 21, 2019

Conversation

Copy link

Member

dalexsoto commentedJun 21, 2019

Adds new classes included in Xcode 11 Beta 1 and 2, all stuff
included in this PR is up to date with Beta 2.

[UIKit] Partial update to Xcode 11 Beta 1 and 2 - Part 1 of ?

daf06e8

Adds new classes included in Xcode 11 Beta 1 and 2, all stuffincluded in this PR is up to date with Beta 2.

dalexsoto requested a review fromspouliot

June 21, 2019 02:44

dalexsoto requested review fromVincentDondain,chamons androlfbjarne ascode owners

June 21, 2019 02:44

dalexsoto mentioned this pull request

Jun 21, 2019

Draft PR for parts of Xcode11 UIKit#6227

Closed

Copy link

Collaborator

monojenkins commentedJun 21, 2019

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, 90 tests passed.

Failed tests

introspection/tvOS - simulator/Debug (tvOS 10.2): Failed

rolfbjarne added3 commits

June 21, 2019 08:07

Fix whitespace.

92d28fd

Add tvOS availability attributes.

5668163

UNNotificationResponse is public, it just need a using.

3a882fa

rolfbjarne requested changes

Jun 21, 2019

View reviewed changes

src/uikit.csShow resolvedHide resolved

src/uikit.cs OutdatedShow resolvedHide resolved

src/uikit.cs

		new bool CanBecomeFocused { get; }

		[NoWatch, NoTV, iOS (11,0)]
		[Watch (5,0), TV (13,0), iOS (11,0)] // Headers state Watch 5.0

Copy link

Member

rolfbjarneJun 21, 2019

Choose a reason for hiding this comment

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

Have you tried running introspection in a watchOS 5 simulator?

src/uikit.cs OutdatedShow resolvedHide resolved

Copy link

Collaborator

monojenkins commentedJun 21, 2019

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, 90 tests passed.

Failed tests

Xtro/Mac: BuildFailure

Update xtro.

ddc5c54

Copy link

MemberAuthor

dalexsoto commentedJun 21, 2019

@rolfbjarne I agree on the naming of the non-static method naming I will revisit them, about the static ones we have theFromFoo Naming pattern everywhere see for examplehttps://docs.microsoft.com/en-us/dotnet/api/uikit.uiimage.frombundle?view=xamarin-ios-sdk-12 so I think keeping the static factory methods being namedFrom is fine and consistent with what we currently do

What do you think?

Copy link

Collaborator

monojenkins commentedJun 21, 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

dalexsoto added2 commits

June 21, 2019 07:44

[tests/.gitignore] Update ignored files

0ed3ef5

Avoid new generated csprojs```tests/apitest/apitest-unified-32.csprojtests/apitest/apitest-unified.csprojtests/apitest/apitest-unifiedXM45-32.csprojtests/apitest/apitest-unifiedXM45.csprojtests/introspection/Mac/introspection-mac-unified-32.csprojtests/introspection/Mac/introspection-mac-unified.csprojtests/linker/mac/dont link/dont link-mac-unified-32.csprojtests/linker/mac/dont link/dont link-mac-unified.csprojtests/linker/mac/dont link/dont link-mac-unifiedXM45-32.csprojtests/linker/mac/dont link/dont link-mac-unifiedXM45.csproj```

Implement feedback

b7bbb7b

spouliot suggested changes

Jun 21, 2019

View reviewed changes

Copy link

Contributor

spouliot left a comment

Choose a reason for hiding this comment

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

minor

src/UIKit/UIEnums.cs OutdatedShow resolvedHide resolved

src/uikit.cs OutdatedShow resolvedHide resolved

src/uikit.cs Outdated

		[Async]
		void PerformSystemAnimation (UISystemAnimation animation, UIView [] views, UIViewAnimationOptions options, Action parallelAnimations, UICompletionHandler completion);

		[TV (12,0), iOS (12,0)] // Yep headers state iOS 12

Copy link

Contributor

spouliotJun 21, 2019

Choose a reason for hiding this comment

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

make sure to run intro tests on an iOS 12.0 simulator :)

Copy link

Member

rolfbjarne commentedJun 21, 2019

@dalexsoto one answer to your question is that past mistakes don't excuse us from doing the right thing in the future 😄

Personally I like theCreate methods (which we've used a lot too), but theFrom* isn't that bad.

@spouliot what do you think?

Copy link

Contributor

spouliot commentedJun 21, 2019

I prefer theCreate (any name with a verb for methods) but we have a lot ofFrom (not the worse non-verb case) and, at least for existing types, we should be consistent.