delba / Permission

A unified API to ask for permissions on iOS
MIT License
2.91k stars 323 forks source link

Swift 3.0 beta 1 #33

Closed hartbit closed 8 years ago

delba commented 8 years ago

Thank you!!

jakerockland commented 8 years ago

@delba @hartbit Are you able to get this to build for XCode 8.0 beta? I'm trying to build with carthage update --platform iOS and am having the build fail with:

The following build commands failed:
    CompileSwift normal arm64
    CompileSwiftSources normal arm64 com.apple.xcode.tools.swift.compiler
hartbit commented 8 years ago

@jakerockland I've opened a pull-request that makes Permission build on beta3 #39

delba commented 8 years ago

@jakerockland It got merged. Thanks @hartbit !

delba commented 8 years ago

Hi @hartbit,

I was wondering why func callbacks(status: PermissionStatus) was renamed func callbackAsync(with: PermissionStatus) ?

hartbit commented 8 years ago

@delba I probably renamed it for several reasons:

delba commented 8 years ago

I got you but it can also be a bit confusing too and lead to think that the difference between callback and callbackAsync is that one is asynchronous and the other isn't. https://github.com/delba/Permission/commit/13b2512ae3c6f23aadbaedef49ed9e780ee2da8c

hartbit commented 8 years ago

Which callback? The property? Well in that case I'm not sure I see the problem. One is a function which is explicitly named as being asynchronous (and it is), and the other one is a closure given at configuration time and which makes no promises of asynchronosity one way or the other.

Perhaps if you want to completely remove any potential ambiguity, we should call the property completionHandler and the function completeAsync. One idea. Perhaps you have another one?

delba commented 8 years ago

In addition of having a similar name, callback and callbackAsync have the same type (Callback which is an alias for PermissionStatus -> Void) and both are passed around the lib hence the risk of confusion. They both are callable even tough one is a property and the other one a method: callback(status) and callbackAsync(status).

The risk is to think that callbackAsync is only this:

func callbackAsync(status: PermissionStatus) {
    async { callback(status) }
}

I initially named it callbacks (plural) because it contains all the callbacks that should be called once the permission has been requested: not only the user defined callback but also the permissionSets delegate method didRequestPermission.

It's not perfect either I agree. Naming is hard ;)

hartbit commented 8 years ago

What do you think of ˋcompletionHandlerand ˋcompleteAsync then?