Skip to content

remove read_phone_state permission #523

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.

Sign up for GitHub

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

Jump to bottom
Merged
AndyScherzinger merged 2 commits into from
Jan 29, 2017
Merged

remove read_phone_state permission #523

AndyScherzinger merged 2 commits into from
Jan 29, 2017

Conversation

@tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Jan 8, 2017 •
edited by AndyScherzinger
Loading

resolves #461

@tobiasKaminsky
Copy link
Member Author

tobiasKaminsky commented Jan 8, 2017

A quick lookup did not reveal the use or need of this permission.
I will put it now into beta.

@AndyScherzinger AndyScherzinger force-pushed the removePhoneStatePermission branch from c85d241 to 28b300b Compare January 10, 2017 11:49
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jan 10, 2017

Both have been added via 9dc51a4 which is part of the SD card PR #92

So I am not sure if they are needed (will have to test). READ_EXTERNAL_STORAGE which is imho already covered (since it is included) via the android.permission.WRITE_EXTERNAL_STORAGE can be removed afaik.

@AndyScherzinger AndyScherzinger force-pushed the removePhoneStatePermission branch from 28b300b to a6ab7dd Compare January 20, 2017 15:08
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jan 20, 2017 •
edited
Loading

@przybylski can you have a look at this? The permission has been added via the SD card support PR, so I am unsure if it can be removed safely.

@AndyScherzinger AndyScherzinger mentioned this pull request Jan 21, 2017
Closed
@AndyScherzinger AndyScherzinger added this to the Nextcloud App 1.4.2 milestone Jan 21, 2017
AndyScherzinger
AndyScherzinger requested changes Jan 21, 2017
View reviewed changes
AndroidManifest.xml Outdated
<uses-permission android:name="android.permission.WAKE_LOCK" />

<uses-permission android:name="android.permission.READ_PHONE_STATE" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
Copy link
Member

@AndyScherzinger AndyScherzinger Jan 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

can imho be removed since android.permission.WRITE_EXTERNAL_STORAGE https://github.com/nextcloud/android/pull/523/files#diff-a48d6def78e18cbabab3c95ae3c9fc49R46 should include this one already.

May remove and push to beta?

@AndyScherzinger AndyScherzinger removed the request for review from przybylski January 21, 2017 21:08
@AndyScherzinger AndyScherzinger force-pushed the removePhoneStatePermission branch from a6ab7dd to 4c8c369 Compare January 26, 2017 10:36
@AndyScherzinger AndyScherzinger force-pushed the removePhoneStatePermission branch from 8d0c446 to 0d080b4 Compare January 27, 2017 11:55
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jan 27, 2017

@mario can you do a review (super minor change 😉)

@AndyScherzinger AndyScherzinger requested a review from mario January 27, 2017 15:20
@AndyScherzinger AndyScherzinger force-pushed the removePhoneStatePermission branch from 0d080b4 to c81e23f Compare January 27, 2017 15:32
@mario
Copy link
Contributor

mario commented Jan 27, 2017

@AndyScherzinger on monday, sorry :-/

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jan 27, 2017

No worries, it's not that urgent!!

Happy weekend :)

@tobiasKaminsky
Copy link
Member Author

tobiasKaminsky commented Jan 27, 2017

as mentioned here #461 (comment) it can be removed

mario
mario approved these changes Jan 29, 2017
View reviewed changes
Copy link
Contributor

@mario mario left a comment

Choose a reason for hiding this comment

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

This review requires a toll of one (1!) pancake. Thank you.

@AndyScherzinger AndyScherzinger merged commit 315986c into master Jan 29, 2017
@AndyScherzinger AndyScherzinger deleted the removePhoneStatePermission branch January 29, 2017 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

+1 more reviewer

@mario mario mario approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

priority: high

Projects

None yet

Milestone

Nextcloud App 1.4.2

Development

Successfully merging this pull request may close these issues.

Android Permissions

4 participants

@tobiasKaminsky @AndyScherzinger @mario