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

Adapt to Swift 3 and add hasPermission method #2

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

natete
Copy link

@natete natete commented Jan 19, 2017

No description provided.

@natete
Copy link
Author

natete commented Jan 19, 2017

I'm not an expert in Swift but it looks like it's working.

The hasPermission method provides a method to check if the permission is granted before requesting it.

If you don't like any of the changes just close the PR, it's something we needed and maybe you can use it.
😄

@akofman
Copy link
Owner

akofman commented Jan 19, 2017

Hey @natete ! Thanks a lot for this work, it looks awesome, I gonna check it and keep you in touch.

@akofman
Copy link
Owner

akofman commented Jan 19, 2017

Okay so I've checked the PermissionScope lib and it seems it already supports Swift 3. I think it would be better to update this plugin with this new last version and its last improvements.

About the hasPermission method, actually the request*Permission methods already check whether the permission is granted before alerting the user. Knowing that do you still think it is convenient to add a hasPermission method ?

Thanks again for using this plugin ;)

@natete
Copy link
Author

natete commented Jan 21, 2017

Sure, it's better to keep in sync with the lib and then adapt only the PermissionScopePlugin file.

Regarding the hasPermission, we implemented it because we were using it in a template pattern using the same workflow for both android and iOS but you can drop it if you consider it.

Thanks to you for creating this awesome plugin!

@akofman
Copy link
Owner

akofman commented Jan 23, 2017

Thx ;) I'm waiting for this PR to be merged then I'll update this plugin.

@akofman akofman mentioned this pull request Aug 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants