Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Use standard cordova PACKAGE_NAME variable in place of applicationId #625

Merged
merged 1 commit into from
Feb 29, 2016
Merged

Use standard cordova PACKAGE_NAME variable in place of applicationId #625

merged 1 commit into from
Feb 29, 2016

Conversation

cfsnyder
Copy link
Contributor

@macdonst
Copy link
Member

@cfsnyder this works and it's a valid way to fix the issue but I'm wondering if we need to merge it as what's there isn't broken and in the new Cordova-Android they add the same fix so we won't need the push.gradle file soon.

apache/cordova-android@fb9cf60

So what benefit does merging this PR give us? I'm not being sarcastic, I'm asking seriously.

@cfsnyder
Copy link
Contributor Author

@macdonst the current push.gradle solution wasn't working in our build process, for whatever reason. I came a across the standard $PACKAGE_NAME variable, found that it fixed my issue and was simpler than the current approach, so I thought I'd submit a PR to suggest the change. If others aren't having issues with the push.gradle solution and Cordova-Android is adding the same fix, I agree that it isn't necessary for you to merge this.

@macdonst
Copy link
Member

@cfsnyder how are you building? Gradle is the future of all the Android stuff.

@cfsnyder
Copy link
Contributor Author

@macdonst we're using the standard gulp-cordova plugins to initiate the build with a gulp task, which means we are using the gradle build system implicitly. I'm not sure if push.gradle was never being run or it may be possible that its output was being overwritten by another stage of the build process. I would have to dive into it further to be sure.

@macdonst
Copy link
Member

@cfsnyder I'm not going to merge this but I do appreciate the pull request. Can you send me more info on your build process as it may be something I can help fix in Gulp Cordova.

@macdonst macdonst closed this Feb 21, 2016
@macdonst macdonst reopened this Feb 29, 2016
@macdonst
Copy link
Member

@cfsnyder well as it turns out this change looks like it will fix #481 so I will be merging.

macdonst added a commit that referenced this pull request Feb 29, 2016
Use standard cordova PACKAGE_NAME variable in place of applicationId
@macdonst macdonst merged commit 64d68bf into phonegap:master Feb 29, 2016
@cfsnyder
Copy link
Contributor Author

@macdonst great glad it could be used.

@macdonst
Copy link
Member

@cfsnyder yeah, it works around a bug in plugman so why not :)

Thanks again for the submission!

@lock
Copy link

lock bot commented Jun 3, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants