Skip to content
This repository has been archived by the owner on Feb 27, 2019. It is now read-only.

Static status methods (except bluetooth & motion) #156

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

Conversation

retendo
Copy link

@retendo retendo commented Feb 26, 2016

Added the ability to get permission statuses (except bluetooth & motion) without creating a PermissionScope instance.
This prevents crashes, when trying to get a status from a different thread than the main thread, as it doesn't try to do any view stuff.

Oliver Roesner added 2 commits February 26, 2016 12:36
…on) without creating a permissionscope instance. this prevents crashes, when trying to get a status from a different thread than the main thread .
@retendo
Copy link
Author

retendo commented Feb 26, 2016

I also added jetbrains product files to the .gitignore.
Sorry for not keeping this separate.

@nickoneill
Copy link
Owner

Hi @retendo, some questions about the motivation for this:

How were you accessing this on a background thread where it was crashing? Does PermissionScope().statusLocationAlways() (or equivalent) not work for you?

And please remove the jetbrains ignores. These should be in your global git ignore, not in the ignores for any project you use.

@retendo
Copy link
Author

retendo commented Feb 26, 2016

There are a few reasons for static status checking methods:

  • Creating an instance of PermissionScope does a lot of view stuff, which shouldn't be done on not-the-main-thread, otherwise you get undefined behavior which can result in crashes. (It did for me)
  • Most of the status checking doesn't need an instance, so why use one?
  • Status checking definitely doesn't need any view stuff to be performed.

@nickoneill
Copy link
Owner

Can you send over an example that crashes when off the main thread? I'm interested if this is our own issue or something particular to native permissions requests.

I totally get where you're coming from in terms of what should and should not be a static. I'm just not sure that shoehorning it in like this is the right solution when (if we can not crash) can do it just as well without.

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

Successfully merging this pull request may close these issues.

2 participants