You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by infil00p <gi...@git.apache.org> on 2017/06/09 22:24:12 UTC
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
GitHub user infil00p opened a pull request:
https://github.com/apache/cordova-android/pull/384
CB-11244: Studio project cleanup
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist
is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
Android
### What does this PR do?
Modernizes Android and makes it so we can do Modern Android Development
### What testing has been done on this change?
* Manual Testing with Plugman
* Dev against e2e tests (upgrade still fails, help plz)
* Talked to Fil regarding JUnit Test running
### Checklist
- [ ] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
- [ ] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
- [ ] Added automated test coverage as appropriate for this change.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/infil00p/cordova-android StudioProjectCleanup
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-android/pull/384.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #384
----
commit 6167130bac0630f8a8359ed491381caa7da45871
Author: Joe Bowser <bo...@apache.org>
Date: 2017-03-30T19:41:44Z
Adding the Studio Builder to build a project based on Android Studio,
and deleting Ant, since Google does not support Ant Builds anymore.
Sorry guys!
commit 2ef15bd2172f265c44b8447a8e2f119d1590fda3
Author: Joe Bowser <bo...@apache.org>
Date: 2017-03-30T20:38:18Z
Moving Android Manifest finding to the Gradle and Studio builders.
commit 09a4ed45ec2c959a5424c85d1e3df04f344eb664
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-03T22:21:48Z
Changing this so we pass lint
commit 5a40dcd86ed8bd21ca611a66ee13170bed099739
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-04T20:38:40Z
Fixing linting issues
commit 3795b65bced7c8634cade5f00bfe2d42349250c9
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-04T22:18:46Z
Setting up the create command so we actually have all the directories in the right place, and define default variables in the top level build.gradle
commit f2396de53e250cd0ad9b8bb2020ce11eaef16ffe
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-04T22:34:42Z
Updating gradle version in the build file
commit 02c07de746b39a587613b531e3fbdcb833d442e9
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-04T22:42:58Z
Managed to get the project to mostly compile, still need to re-work the build command to add the app project
commit c8f328fe2a9b6d05ad543b32989182bb612e9d0a
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-11T20:47:40Z
Made changes so cordova/build builds with the new project. Need to work on plugin installation.
commit 7db18f8b361369d10237f5159154dbae3f0c3f4d
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-11T21:41:27Z
Fix the overwriting of Fil's fix, blargh
commit cc2d1fd9ae59e5aca10ff5271ef637b6b6897eee
Author: Joe Bowser <bo...@apache.org>
Date: 2017-01-04T19:48:18Z
CB-11244: Changing directory creation, will most likely hide this behind a flag for the next release of Cordova-Android, and then make it default in the next major pending feedback
commit 844dc7f3df7f830cf562535ef9f5f5fc15471ca1
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-18T21:29:02Z
Changing the project to add the app directory as a dependency
commit 2df146df4f918067eb325ca1d50639045f5003ed
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-18T21:48:56Z
JsHint Fixes, deleting unused methods
commit c1ea8ec88576586a7385dd06d5d62897f9142b4e
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-19T18:50:55Z
Fixed the specification of the builders in the run command by getting build to check what was being passed from run
commit 17416dfd1df16da9b4c0715ec6280961c7b9d91b
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-21T23:27:26Z
Fixing the Android Studio detection and making it automatically pick the right builder, good for upgrading Cordova
commit e30968e15a8aadbc6cc20d4b483f27f1c580ac15
Author: Joe Bowser <bo...@apache.org>
Date: 2017-04-22T00:02:27Z
Updated AndroidStudio to only look for the app directory to determine studio status
commit a17a5b891f98417d149bacbf8307412fa8c03a34
Author: Joe Bowser <bo...@apache.org>
Date: 2017-05-31T17:23:35Z
CB-11244: Setup Api.js to support multiple builders based on project structure
commit a5ca7089eaa2e8825dca9f7c2140c2f62b5d618c
Author: Joe Bowser <bo...@apache.org>
Date: 2017-05-31T17:37:47Z
Fixing lint errors
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by imhotep <gi...@git.apache.org>.
Github user imhotep commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123850564
--- Diff: bin/templates/cordova/Api.js ---
@@ -343,6 +349,8 @@ Api.prototype.removePlugin = function (plugin, uninstallOptions) {
*/
Api.prototype.build = function (buildOptions) {
var self = this;
+ if(this.android_studio)
+ buildOptions.studio = true;
--- End diff --
I think it makes to have abstracted somewhere else as well
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android issue #384: CB-11244: Studio project cleanup
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on the issue:
https://github.com/apache/cordova-android/pull/384
We're going to keep working on #389, closing this old branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by imhotep <gi...@git.apache.org>.
Github user imhotep commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123851340
--- Diff: bin/lib/create.js ---
@@ -259,41 +273,53 @@ exports.create = function(project_path, config, options, events) {
setShellFatal(true, function() {
var project_template_dir = options.customTemplate || path.join(ROOT, 'bin', 'templates', 'project');
+ var app_path = path.join(project_path, 'app', 'src', 'main');
--- End diff --
Is this default? do you plan on having a way to create old project structure still? or is the plan to just support managing the old&existing structures while only being able to create the new (Android Studio) structure?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r124071629
--- Diff: bin/lib/create.js ---
@@ -259,41 +273,53 @@ exports.create = function(project_path, config, options, events) {
setShellFatal(true, function() {
var project_template_dir = options.customTemplate || path.join(ROOT, 'bin', 'templates', 'project');
+ var app_path = path.join(project_path, 'app', 'src', 'main');
--- End diff --
The plan is to support managing the old and existing structures while only being able to create new structures for the time being. Of course, there's a debate on how update works, and we may just toss the structure altogether. Who knows?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p closed the pull request at:
https://github.com/apache/cordova-android/pull/384
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123834356
--- Diff: bin/lib/create.js ---
@@ -125,16 +131,24 @@ function writeProjectProperties(projectPath, target_api) {
fs.writeFileSync(dstPath, data);
}
-function prepBuildFiles(projectPath) {
+// This makes no sense, what if you're building with a different build system?
--- End diff --
Would folding this functionality up into the Builder abstraction make sense here? Would different project structures require different building different things at create time as well? Put otherwise: do the Builders have to do specific things at create-time as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by imhotep <gi...@git.apache.org>.
Github user imhotep commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123850098
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -148,6 +152,7 @@ module.exports.runClean = function(options) {
*/
module.exports.run = function(options, optResolvedTarget) {
var opts = parseOpts(options, optResolvedTarget, this.root);
+ console.log(opts.buildMethod);
--- End diff --
another one :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android issue #384: CB-11244: Studio project cleanup
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/cordova-android/pull/384
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=h1) Report
> Merging [#384](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/3a6e898b12eac31dfc16a4c526dfba8fab158723?src=pr&el=desc) will **decrease** coverage by `0.29%`.
> The diff coverage is `20.58%`.
[](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #384 +/- ##
=========================================
- Coverage 39.52% 39.22% -0.3%
=========================================
Files 16 16
Lines 1551 1555 +4
Branches 277 279 +2
=========================================
- Hits 613 610 -3
- Misses 938 945 +7
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/builders/builders.js](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9idWlsZGVycy5qcw==) | `33.33% <ø> (ø)` | :arrow_up: |
| [...n/templates/cordova/lib/builders/GenericBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9HZW5lcmljQnVpbGRlci5qcw==) | `28.84% <0%> (-1.3%)` | :arrow_down: |
| [bin/templates/cordova/lib/build.js](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZC5qcw==) | `10.13% <0%> (-0.21%)` | :arrow_down: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `18.18% <0%> (ø)` | :arrow_up: |
| [bin/templates/cordova/lib/AndroidStudio.js](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9BbmRyb2lkU3R1ZGlvLmpz) | `94.73% <100%> (ø)` | :arrow_up: |
| [...in/templates/cordova/lib/builders/GradleBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9HcmFkbGVCdWlsZGVyLmpz) | `19.62% <15.78%> (-0.38%)` | :arrow_down: |
| [bin/templates/cordova/Api.js](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL0FwaS5qcw==) | `42.37% <33.33%> (-0.61%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=footer). Last update [3a6e898...49b76f5](https://codecov.io/gh/apache/cordova-android/pull/384?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123834608
--- Diff: bin/lib/create.js ---
@@ -314,7 +340,18 @@ exports.update = function(projectPath, options, events) {
return Q()
.then(function() {
- var manifest = new AndroidManifest(path.join(projectPath, 'AndroidManifest.xml'));
+ var isAndroidStudio = AndroidStudio.isAndroidStudioProject(projectPath);
+ var isLegacy = !isAndroidStudio;
+ var manifest = null;
+ var builder = 'gradle';
+
+ if(isAndroidStudio) {
+ manifest = new AndroidManifest(path.join(projectPath, 'app', 'main', 'AndroidManifest.xml'));
+ builder = 'studio';
+ } else {
+ manifest = new AndroidManifest(path.join(projectPath, 'AndroidManifest.xml'));
+ builder = 'gradle';
+ }
--- End diff --
Same thing here, this may be better folded up into the Builders?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by imhotep <gi...@git.apache.org>.
Github user imhotep commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123849628
--- Diff: bin/templates/cordova/lib/builders/GenericBuilder.js ---
@@ -53,43 +48,13 @@ GenericBuilder.prototype.findOutputApks = function(build_type, arch) {
var self = this;
return Object.keys(this.binDirs)
.reduce(function (result, builderName) {
+ console.log('builderName:'+ builderName);
--- End diff --
don't forget this in there ;-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123844605
--- Diff: bin/lib/create.js ---
@@ -125,16 +131,24 @@ function writeProjectProperties(projectPath, target_api) {
fs.writeFileSync(dstPath, data);
}
-function prepBuildFiles(projectPath) {
+// This makes no sense, what if you're building with a different build system?
--- End diff --
We're already doing that in the next two lines?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request #384: CB-11244: Studio project cleanup
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/384#discussion_r123835148
--- Diff: bin/templates/cordova/Api.js ---
@@ -81,10 +83,13 @@ function Api(platform, platformRootDir, events) {
// XXX Override some locations for Android Studio projects
if(AndroidStudio.isAndroidStudioProject(self.root) === true) {
selfEvents.emit('log', 'Android Studio project detected');
+ this.builder='studio';
this.android_studio = true;
this.locations.configXml = path.join(self.root, 'app/src/main/res/xml/config.xml');
- this.locations.strings = path.join(self.root, 'app/src/main/res/xml/strings.xml');
+ this.locations.strings = path.join(self.root, 'app/src/main/res/values/strings.xml');
this.locations.manifest = path.join(self.root, 'app/src/main/AndroidManifest.xml');
+ //We could have Java Source, we could have other languages
+ this.locations.javaSrc = path.join(self.root, 'app/src/main/java/');
--- End diff --
Same sort of deal, we could push the responsibility for figuring out the locations to the builders, rather than have the Api function worry about each possible implementation of a project structure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org