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

Removed requirement of Android hooks #64

Merged
merged 1 commit into from
Aug 30, 2019
Merged

Removed requirement of Android hooks #64

merged 1 commit into from
Aug 30, 2019

Conversation

sagrawal31
Copy link
Collaborator

@dpa99c your thoughts on this, please.

@dpa99c
Copy link
Owner

dpa99c commented Jul 27, 2019

@sagrawal31 Thanks, I'll give this a test and report back.

FYI I'm moving house next week so fairly busy packing my entire life into boxes right now but I will try to get back to you ASAP 😀

@dpa99c
Copy link
Owner

dpa99c commented Aug 6, 2019

As I've outlined in #63, I've determined the cause of the uninstall issue and it's not related to the Android hook scripts removed by this PR.
Therefore I'm not sure merging this PR is necessary unless it adds other benefits?

@dpa99c
Copy link
Owner

dpa99c commented Aug 16, 2019

@sagrawal31 Is there any benefit to merging this PR now (since #63 was fixed by removing the <resource-file> to copy the dummy google-services.json to the Android project)?

I guess it's cleaner than using the hook scripts but I'm always nervous of using the apply plugin in non-root Gradle configs to due the possibility of version conflicts with other plugins...

@sagrawal31
Copy link
Collaborator Author

@dpa99c In my view, using custom gradle file will keep the repository clean and readable as Android developers (who even have bare knowledge) will be able to understand the gradle file.

But if you feel, there might be a chance in future of conflict then take the call and close this MR. We can revisit this in the future if needed.

dpa99c added a commit that referenced this pull request Aug 30, 2019
Removed requirement of Android hooks.
@dpa99c dpa99c merged commit 531bfe5 into master Aug 30, 2019
@sagrawal31
Copy link
Collaborator Author

Thanks @dpa99c 😄

@dpa99c
Copy link
Owner

dpa99c commented Aug 30, 2019

@sagrawal31 No worries - I had another look at this and decided your approach is less fragile than those hook scripts. If any issues arise due to the extra Gradle config, we'll tackle them as they come up.
I'm back from vacation/moving house now so I'll try to address the outstanding issues and do a new patch release in the next few days.

@dpa99c dpa99c deleted the android-cleanup branch August 30, 2019 10:23
@sagrawal31
Copy link
Collaborator Author

I'm glad our thoughts are in sync. 👍

By the way, I'm also back from vacation/moving to a new house.

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