Make SDK API more Swift-friendly #2201

DmitriyKirakosyan · 2020-10-21T13:17:00Z

Things to consider before you submit the PR:

Has CHANGELOG.md been updated?
Are tests passing locally?
Are the files formatted correctly?
~~[ ] Did you add unit tests?~~
~~[ ] Did you check UI tests on the sample app? They are not executed on CI.~~
Did you test your change with either the sample apps that are included in the repository or with a blank app that uses your change?

Description

Updates SDK Swift API to make it more swifty.

NOTE: I's a breaking change!

Related PRs or issues

* Changes in core module headers * Make properties of class * Make property out of enabled; Add swift class name * Change logLevel to property * Rearrange methods (to keep properties together) * Fix format * Add swift-friendly names * Add property for logUrl * Address PR feedbacks * Add swift-friendly overloads * Change swift signatures * Convert the sets methods to properties. * Reverted setCustomProperties changes * Remove setters definition from properties Co-authored-by: Anastasia Kubova <v-ankubo@microsoft.com> Co-authored-by: Dima <v-dmkira@microsoft.com>

* Swiftify class names in Distribute module * Swiftify methods in Distribute module * Swiftify some properties * [Distribute] Convert public setters to properties * Remove redundant NS_SWIFT_NAME * Addressed PR notes Co-authored-by: Ivan Matkov <v-ivmatk@microsoft.com>

* Changes in core module headers * Make properties of class * Make property out of enabled; Add swift class name * Change logLevel to property * Rearrange methods (to keep properties together) * Fix format * Add swift-friendly names * Add property for logUrl * Add swifty names to Crashes module * Address PR feedbacks * Add swift-friendly overloads * Add missing changes * Resolve feedbacks * Formatting the property * Fix naming * Resolve feedbacks * Fix test Co-authored-by: Dmitrii Chichuk <v-dmchic@microsoft.com>

* Changes in core module headers * Make properties of class * Make property out of enabled; Add swift class name * Change logLevel to property * Rearrange methods (to keep properties together) * Fix format * Add swift-friendly names * Swiftify Analytics module * Swiftify setters in Analytics * Remove setters definition from properties Co-authored-by: Dmitrii Chichuk <v-dmchic@microsoft.com> Co-authored-by: Anastasia Kubova <v-ankubo@microsoft.com>

Co-authored-by: Ivan Matkov <v-ivmatk@microsoft.com>

* Update iOS apps Co-authored-by: Anastasia Kubova <v-ankubo@microsoft.com> Co-authored-by: Dmitriy Kirakosyan <v-dmkira@microsoft.com> Co-authored-by: Dmitrii Chichuk <v-dmchic@microsoft.com> * Update macOS apps Co-authored-by: Anastasia Kubova <v-ankubo@microsoft.com> Co-authored-by: Dmitriy Kirakosyan <v-dmkira@microsoft.com> Co-authored-by: Dmitrii Chichuk <v-dmchic@microsoft.com> * Update tvOS apps Co-authored-by: Anastasia Kubova <v-ankubo@microsoft.com> Co-authored-by: Dmitriy Kirakosyan <v-dmkira@microsoft.com> Co-authored-by: Dmitrii Chichuk <v-dmchic@microsoft.com> * Update comments in test apps * Update some used API for core and crashes modules Co-authored-by: Dmitriy Kirakosyan <v-dmkira@microsoft.com> Co-authored-by: Dmitrii Chichuk <v-dmchic@microsoft.com>

AppCenter/AppCenter/Internals/Context/Device/MSACDeviceTracker.h

# Conflicts: # Sasquatch/SasquatchSwift/AppCenterDelegateSwift.swift # Sasquatch/SasquatchSwift/AppDelegate.swift # SasquatchMac/SasquatchMacSwift/AppCenterDelegateSwift.swift # SasquatchMac/SasquatchMacSwift/AppDelegate.swift

* Fixed property typo and removed redundant setter * Added getCrashHandlerSetupDelegate back and marked deprecated

iamclement

Naming is not easy thanks for looking at this :). I'll take another look couldn't get to the end.

AppCenter/AppCenter/MSACCustomProperties.h

AppCenter/AppCenter/MSACEnable.h

AppCenter/AppCenter/Internals/Context/Device/MSACDeviceTracker.m

AppCenter/AppCenter/Internals/Ingestion/MSACIngestionProtocol.h

AppCenter/AppCenter/MSACAppCenter.h

Co-authored-by: Clement Polet <clpolet@microsoft.com>

DmitriyKirakosyan · 2020-10-27T11:46:29Z

The public delegates (e.g. MSACCrashesDelegate) have more complicated API names I feel like we should do a pass on them. Particularly paying attention to omit words that merely repeat type information.
Other than that I have seen all the files I wanted to review :).

Regarding the delegates, I think swift api already looks fine. Eg:

- (void)crashes:(MSACCrashes *)crashes didFailSendingErrorReport:(MSACErrorReport *)errorReport withError:(NSError *)error;

Resolves to swift as

func crashes(_ crashes: Crashes, didFailSending errorReport: ErrorReport, withError error: Error) {
}

Here is the place in swift code https://github.com/microsoft/appcenter-sdk-apple/blob/develop/Sasquatch/SasquatchSwift/AppDelegate.swift#L217

Do you think there still should be some additional swiftify changes applied?

Sasquatch/SasquatchSwift-Extension/ExtensionViewController.swift

iamclement · 2020-10-27T16:58:16Z

The public delegates (e.g. MSACCrashesDelegate) have more complicated API names I feel like we should do a pass on them. Particularly paying attention to omit words that merely repeat type information.
Other than that I have seen all the files I wanted to review :).

Regarding the delegates, I think swift api already looks fine. Eg:
- (void)crashes:(MSACCrashes *)crashes didFailSendingErrorReport:(MSACErrorReport *)errorReport withError:(NSError *)error;
Resolves to swift as
func crashes(_ crashes: Crashes, didFailSending errorReport: ErrorReport, withError error: Error) {
}
Here is the place in swift code https://github.com/microsoft/appcenter-sdk-apple/blob/develop/Sasquatch/SasquatchSwift/AppDelegate.swift#L217

Do you think there still should be some additional swiftify changes applied?

This one should be fine but have you checked them all? I saw one yesterday that explicitly labeled ErrorReport. I'll double check.
Ah this one: func crashes(_ crashes: Crashes!, shouldProcessErrorReport errorReport: ErrorReport!) -> Bool

Co-authored-by: Ivan Matkov <v-ivmatk@microsoft.com>

iamclement

One thing, probably the last :). error or withError somehow is always explicitly labeled (e.g. NSApplicationdelegate, NSURLSessionDelegate)

DmitriyKirakosyan · 2020-10-28T12:40:51Z

One thing, probably the last :). error or withError somehow is always explicitly labeled (e.g. NSApplicationdelegate, NSURLSessionDelegate)

Reverted the previous state for these methods, coz swift default conversion did it correctly. The only method that didn't translate automatically was func crashes(_ crashes: Crashes!, shouldProcessErrorReport errorReport: ErrorReport!) -> Bool, so I fixed it. Now all delegate method signatures are converted correctly in swift : https://github.com/microsoft/appcenter-sdk-apple/blob/develop/Sasquatch/SasquatchSwift/AppDelegate.swift#L211-L220

Also removed warning in sasquatch apps.

And converted MSACLogHandler and MSACLogMessageProvider to the short versions for swift.

MatkovIvan · 2020-10-28T12:44:00Z

AppCenterCrashes/AppCenterCrashes/MSACCrashesDelegate.h


 NS_ASSUME_NONNULL_END
+
+@end


Move before NS_ASSUME_NONNULL_END

iamclement

LGTM

AppCenter/AppCenter/Internals/Context/Device/MSACDeviceTracker.h

Dmitrii Chichuk and others added 13 commits October 20, 2020 12:26

Resolve feedbacks

61486b9

Reverted is and has swififyings

6fdc150

Convert more getters to properties

4a4d279

Update AppCenter/AppCenter/Internals/Ingestion/MSACIngestionProtocol.h

84dbf88

Co-authored-by: Ivan Matkov <v-ivmatk@microsoft.com>

Updated CHANGELOG

6409078

Updated changelog

db78f89

Fix mandatoryUpdate property

f8c500e

Add proper migration links to Changelog

c3ec774

DmitriyKirakosyan requested review from MatkovIvan, iamclement and AnastasiaKubova October 21, 2020 13:17

This comment has been minimized.

Sign in to view

MatkovIvan suggested changes Oct 21, 2020

View reviewed changes

AppCenter/AppCenter/Internals/Context/Device/MSACDeviceTracker.h Outdated Show resolved Hide resolved

DmitriyKirakosyan added 4 commits October 21, 2020 16:25

Merge branch 'develop' into feature/swiftify-api

0739003

# Conflicts: # Sasquatch/SasquatchSwift/AppCenterDelegateSwift.swift # Sasquatch/SasquatchSwift/AppDelegate.swift # SasquatchMac/SasquatchMacSwift/AppCenterDelegateSwift.swift # SasquatchMac/SasquatchMacSwift/AppDelegate.swift

Moved getters to private file. Removed get prefix in getters.

7f0e861

Format NSACDeviceTracker.m

5e67663

Swiftify Enums, Update Demo apps

3d1db5f

AnastasiaKubova force-pushed the feature/swiftify-api branch from e9fa4bc to 3d1db5f Compare October 22, 2020 05:33

DmitriyKirakosyan requested a review from MatkovIvan October 22, 2020 07:15

DmitriyKirakosyan added 2 commits October 22, 2020 15:04

Fix crashHandlerSetupDelegate property

bfe2e12

* Fixed property typo and removed redundant setter * Added getCrashHandlerSetupDelegate back and marked deprecated

Fix tests

6fe9264

Jamminroot previously approved these changes Oct 22, 2020

View reviewed changes

iamclement suggested changes Oct 22, 2020

View reviewed changes

vvechkanov previously approved these changes Oct 22, 2020

View reviewed changes

Addressed review notes

eff559c

Update CHANGELOG.md

5f1c1d0

Co-authored-by: Clement Polet <clpolet@microsoft.com>

DmitriyKirakosyan added 3 commits October 27, 2020 14:52

Add weak to MSACCrashesDelegate property

d402789

Remove prefix of MSACUserConfirmationHandler

7ef96f7

Remove _Nullable's in .m and .mm files

5c2ead1

MatkovIvan previously approved these changes Oct 27, 2020

View reviewed changes

Sasquatch/SasquatchSwift-Extension/ExtensionViewController.swift Outdated Show resolved Hide resolved

Update Sasquatch/SasquatchSwift-Extension/ExtensionViewController.swift

676944a

Co-authored-by: Ivan Matkov <v-ivmatk@microsoft.com>

DmitriyKirakosyan dismissed MatkovIvan’s stale review via 676944a October 27, 2020 20:09

DmitriyKirakosyan added 2 commits October 27, 2020 23:51

Swiftify delegates

2da7533

Fix sasquatches

a4fb656

MatkovIvan previously approved these changes Oct 28, 2020

View reviewed changes

iamclement suggested changes Oct 28, 2020

View reviewed changes

DmitriyKirakosyan added 4 commits October 28, 2020 15:23

Fix delegates. Reverted redundant NS_SWIFT_NAME's

0ac4774

Fix sasquatch apps

0f85387

Swiftify MSACLogMessageProvider and MSACLogHandler

1f1c0d0

Fix warning in tv and mac sasquatches

c506101

DmitriyKirakosyan dismissed MatkovIvan’s stale review via c506101 October 28, 2020 12:35

MatkovIvan reviewed Oct 28, 2020

View reviewed changes

AppCenterCrashes/AppCenterCrashes/MSACCrashesDelegate.h

NS_ASSUME_NONNULL_END

@end

Copy link

Contributor

MatkovIvan Oct 28, 2020

Choose a reason for hiding this comment

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

Move before NS_ASSUME_NONNULL_END

DmitriyKirakosyan added 2 commits October 28, 2020 15:47

Reverted NS_ASSUME_NONNULL_END position

9a2b9d9

Fix formatting

c212b71

iamclement previously approved these changes Oct 28, 2020

View reviewed changes

MatkovIvan reviewed Oct 28, 2020

View reviewed changes

AppCenter/AppCenter/Internals/Context/Device/MSACDeviceTracker.h Outdated Show resolved Hide resolved

Update AppCenter/AppCenter/Internals/Context/Device/MSACDeviceTracker.h

5ab4229

DmitriyKirakosyan dismissed iamclement’s stale review via 5ab4229 October 28, 2020 16:46

MatkovIvan approved these changes Oct 28, 2020

View reviewed changes

MatkovIvan merged commit 36d98b9 into develop Oct 28, 2020

MatkovIvan deleted the feature/swiftify-api branch October 28, 2020 17:27

vvechkanov mentioned this pull request Nov 23, 2020

Make the API more Swifty #1627

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make SDK API more Swift-friendly #2201

Make SDK API more Swift-friendly #2201

DmitriyKirakosyan commented Oct 21, 2020 •

edited by xabldint

This comment has been minimized.

iamclement left a comment

DmitriyKirakosyan commented Oct 27, 2020

iamclement commented Oct 27, 2020 •

edited

iamclement left a comment

DmitriyKirakosyan commented Oct 28, 2020

MatkovIvan Oct 28, 2020

iamclement left a comment

Make SDK API more Swift-friendly #2201

Make SDK API more Swift-friendly #2201

Conversation

DmitriyKirakosyan commented Oct 21, 2020 • edited by xabldint

Description

Related PRs or issues

This comment has been minimized.

iamclement left a comment

Choose a reason for hiding this comment

DmitriyKirakosyan commented Oct 27, 2020

iamclement commented Oct 27, 2020 • edited

iamclement left a comment

Choose a reason for hiding this comment

DmitriyKirakosyan commented Oct 28, 2020

MatkovIvan Oct 28, 2020

Choose a reason for hiding this comment

iamclement left a comment

Choose a reason for hiding this comment

DmitriyKirakosyan commented Oct 21, 2020 •

edited by xabldint

iamclement commented Oct 27, 2020 •

edited