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

Merge pull with updates from API level 21 to API level 28 #117

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

Merge pull with updates from API level 21 to API level 28 #117

wants to merge 12 commits into from

Conversation

MarcoTTC
Copy link

Hi. You may not know me, but my name is Marco, and I'm trying to make a break in my career as a mobile software developer. Thus, I've decided to contribute to some open source GitHub mobile projects I've seen around. I've chosen SoundRecorder as my first try, and I've updated the project to build and deploy as a target API level 28 android project, modifying the code whenever needed to make it so. I've tried to make my commits small and didactic as possible. Since I put some effort into making sure the code compiles, I hope you can appreciate my changes to the code, and accept the merge pull request, but if you could also launch a new version on Google Play, that would be great. Also, I hope to add this project to my portfolio as something I've contributed code towards to, respecting the original authorship. Thank you for your time, and I hope you like my efforts with SoundRecorder.

Copy link

@naXa777 naXa777 left a comment

Choose a reason for hiding this comment

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

  1. You request permissions at runtime in RecordFragment, but don't do this in PlaybackFragment. Let's assume that permissions for recording (MIC + STORAGE) were granted and then revoked. In this case, when user goes to recordings tab and plays a recording, what will happen? Does the app handle this case properly?
  2. Android Pie requires a special permission and a persistent notification for sound recording in background. Please verify this use case as well.

In general, you've done good work! You just need to test the mentioned use cases and apply changes if required.

@MarcoTTC
Copy link
Author

Thanks for mentioning the special audio permission and notification for Android Pie. I'll make sure to double check the proper audio requisition and usage in Pie. Also, I may be mistaken, but I think only external data write permission and microphone recording are considered dangerous by Android standards, while audio playback and external data reading are not, but I'll double check it too and implement the proper changes when I have the time. Thank you again.

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