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

Make the API more Swifty #1627

Closed
sindresorhus opened this issue May 17, 2019 · 11 comments
Closed

Make the API more Swifty #1627

sindresorhus opened this issue May 17, 2019 · 11 comments

Comments

@sindresorhus
Copy link

The API could be more user-friendly for Swift users.

Some quick examples of what I would recommend:

-MSCrashes.hasCrashedInLastSession()
+MSCrashes.hasCrashedInLastSession
-MSCrashes.setDelegate(self)
+MSCrashes.delegate = self
-func crashes(_ crashes: MSCrashes!, willSend errorReport: MSErrorReport!) {
+func crashes(_ crashes: MSCrashes, willSend errorReport: MSErrorReport) {
-MSCrashes.disableMachExceptionHandler()
-MSAppCenter.start("{Your App Secret}", withServices: [MSCrashes.self])
+MSAppCenter.start("{Your App Secret}", withServices: [MSCrashes.self], options: [.withoutMachExceptionHandler])
-MSCrashes.setEnabled(false)
-MSCrashes.setEnabled(true)
-MSCrashes.isEnabled()
+MSCrashes.isEnabled = false
+MSCrashes.isEnabled = true
+MSCrashes.isEnabled

More here: https://developer.apple.com/documentation/swift/objective-c_and_c_code_customization/improving_objective-c_api_declarations_for_swift

@annakocheshkova
Copy link

@sindresorhus thanks for the suggestion. We are always open to the ways to improve the SDK.

@jaeklim
Copy link
Contributor

jaeklim commented May 21, 2019

@sindresorhus I appreciate your suggestion for Swift methods of App Center SDK. Unfortunately, it is not in our immediate plan to support it. We will keep this thread open for developers to comment and vote, and keep you posted once we decide to work on it.

@tonyarnold
Copy link
Contributor

@jaeklim none of @sindresorhus has suggested requires you to use Swift directly.

This can all be done by properly structuring your Objective-C API using properties (both instance and class properties). I'll draw up a PR with at least what @sindresorhus has suggested above as an exemplar.

@Jamminroot
Copy link
Contributor

@tonyarnold sure, feel free to create PR.

@iamclement
Copy link
Contributor

Thanks for bumping up the topic and submitting a PR @tonyarnold . This is indeed some good improvements we can take, we are welcoming contributions. We’ll review the PR. FYI among other thing we are looking at to approve a PR is the consistency between modules particularly because this topic impacts all modules. If any breaking change is to be added it should be added in a separate PR because it’ll probably stack up until the next SDK major version will show up. If we think we need to iterate on this we can create feature branches.

@AnastasiaKubova
Copy link

The feature has been released in 4.0.0 version of App Center.

@tonyarnold
Copy link
Contributor

Please pass on my thanks to your team, @AnastasiaKubova 😁

@sindresorhus
Copy link
Author

Really nice work on this everyone.


I did notice one thing though. It went from MSAppCenter.isEnabled() to AppCenter.enabled. This should be AppCenter.isEnabled.

@vvechkanov
Copy link
Contributor

Hi @sindresorhus!
We are glad that you like our work! What about your note we were considering both options and decide to write it this way. Here you can found our PR, with discussion. The main idea was consistency between ObjC and Swift code. We use AppCenter.enabled as a name for property and is_ and set_ as prefixes for getters and setters.

@sindresorhus
Copy link
Author

The whole point was to swiftify the API, meaning less consistency between Swift and Objective-C. Why not do it 100%?

@DmitriyKirakosyan
Copy link
Contributor

DmitriyKirakosyan commented Dec 21, 2020

@sindresorhus, we considered and discussed it thoroughly within the team while doing these changes, and it was decided to use enabled in Swift as this property is mutable.

I'm sorry to inform you that, but we are not planning to do any updates related to Swift-friendly API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants