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

Fix issue 2705 release and debug builds can now coexist #2743

Closed
wants to merge 0 commits into from

Conversation

@prateek3255
Copy link
Collaborator

commented Jun 29, 2018

This PR is targeted to solve the issue #2705.

I updated the build.gradle and AndroidManifest.xml file according to these docs and this stackoverflow answer.

The update in the build.gradle file will append .debug in the package name for debug builds and the change in AndroidManifest.xml file was done to avoid the duplicate permission error due to the release app already being installed on the device, using ${applicationId} will dynamically change the package name for permissions.

I have tested the changes locally and installed both the release and debug builds simultaneously on the device successfully here is the gif for the same -

zuli

@jainkuniya
Copy link
Member

left a comment

@prateek3255 👍
Will check it out tonight.

@jainkuniya
Copy link
Member

left a comment

Awesome work @prateek3255 👍 Keep it up!
I tried this and face into one problem, of having same app icon for both the build. It was kinda creating confusion. I have taken this commit & added one more and submitted #2745. :)
Now we can close this :)

@jainkuniya

This comment has been minimized.

Copy link
Member

commented Jul 1, 2018

I updated the build.gradle and AndroidManifest.xml file according to these docs and this stackoverflow answer.

And glad that you mentioned the references too 👍

@prateek3255

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2018

Thanks, @jainkuniya,
But will my contribution still be counted if I close this PR?

@gnprice

This comment has been minimized.

Copy link
Member

commented Jul 1, 2018

Thanks @prateek3255 -- this is great!

There's one bit of trouble I'm having as I play with it; and thanks @jainkuniya for the modified icon, as it helps in noticing the difference.

That's that when I run react-native run-android, it goes and builds the app just fine and installs it... and then what it starts up is the release version, not the debug version I've just built.

You can see this in the output:

:app:installDebug
Installing APK 'app-debug.apk' on 'Virtual_Pixel_2_XL_API_27(AVD) - 8.1.0' for app:debug
Installed on 1 device.

BUILD SUCCESSFUL

Total time: 6.74 secs
Running adb -s emulator-5554 reverse tcp:8081 tcp:8081
Starting the app on emulator-5554 (adb -s emulator-5554 shell am start -n com.zulipmobile/com.zulipmobile.MainActivity)...
Starting: Intent { cmp=com.zulipmobile/.MainActivity }

Note it says com.zulipmobile, not com.zulipmobile.debug.

You can also see this because when using #2745, if you hit the switch-app button, the app that was in the foreground has the unmodified icon.

It seems like we need to teach installDebug that it should start the debug version. Would you work out how to do that too, and add that to this PR?

@gnprice

This comment has been minimized.

Copy link
Member

commented Jul 1, 2018

Also, re workflow: I'm happy to leave this PR open, and when it's ready end up merging both it and #2745 .

It'll be the same result in Git either way, because with our review/merge workflow, the individual commits in a PR branch each become commits in master.

@jainkuniya

This comment has been minimized.

Copy link
Member

commented Jul 2, 2018

Nothing extra needs to be done for launching debug build,
just use react-native run-android --appIdSuffix=debug

:app:installDebug
Installing APK 'app-debug.apk' on 'Samsung Galaxy S5 - 4.4.4 - API 19 - 1080x1920 - 4.4.4' for app:debug
Installed on 1 device.

BUILD SUCCESSFUL

Total time: 39.0 secs
Running adb -s 192.168.56.101:5555 reverse tcp:8081 tcp:8081
error: closed
Could not run adb reverse: Command failed: adb -s 192.168.56.101:5555 reverse tcp:8081 tcp:8081
Starting the app on 192.168.56.101:5555 (adb -s 192.168.56.101:5555 shell am start -n com.zulipmobile.debug/com.zulipmobile.MainActivity)...
Starting: Intent { cmp=com.zulipmobile.debug/com.zulipmobile.MainActivity }
Vishweshs-MacBook-Pro:zulip-mobile vishwesh$ 

Updating this in the commit message & in our docs.

@gnprice

This comment has been minimized.

Copy link
Member

commented Jul 5, 2018

Thanks for finding that solution! See discussion in #2745.

@zulipbot

This comment has been minimized.

Copy link
Member

commented Jul 30, 2018

Heads up @prateek3255, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

@gnprice gnprice closed this Aug 1, 2018

@gnprice gnprice force-pushed the prateek3255:fix/2705 branch from 7e03047 to 2857d59 Aug 1, 2018

@gnprice

This comment has been minimized.

Copy link
Member

commented Aug 1, 2018

This was merged yesterday as 2857d59. Thanks again @prateek3255 !

@prateek3255

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 1, 2018

Thanks, 😃 @gnprice can you please also review my PR #2804 for the splash screen issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.