[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-20 Thread filmaj 
Github user filmaj commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
Rebased on latest master, will wait for appveyor to pass before merging.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-20 Thread purplecabbage 
Github user purplecabbage commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
LGTM!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-20 Thread dpogue 
Github user dpogue commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
The `android_sdk_version` issue doesn't seem to cause any problems with 
building, and this PR does work for building with both the older and latest 
SDKs, so that's a :+1: from me


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-20 Thread filmaj 
Github user filmaj commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
I've posted a DISCUSS for removal of the `android_sdk_version` script here: 
http://markmail.org/message/k4oysup6lkfzk4o2

Any opposition to me merging it in? I am hesitant to do so without an 
explicit +1 from some other committers.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-16 Thread shazron 
Github user shazron commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
@filmaj as usual, perhaps we need to keep it around, and set a deprecation 
period of three releases (we had a discussion in dev@)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-16 Thread filmaj 
Github user filmaj commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
@shazron I think that's expected and will happen if you also try it with 
the `master` branch. It is because `android_sdk_version` does not leverage 
`check_reqs`, which will modify your `PATH` and `ANDROID_HOME` environment 
variables.

I can update it, though, to use `check_reqs`? It would then work unless you 
have _neither_ `ANDROID_HOME` _nor_ any of the Android SDK tooling on your 
`PATH`.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-16 Thread shazron 
Github user shazron commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
I have the same results as @dpogue. Tested macOS Sierra with Android SDK 
25.3.1 and Windows 10 with Android SDK 25.3.1.

For `android_sdk_version` on Windows I got this error:
```
> bin\android_sdk_version
{ [Error: cmd: Command failed with exit code 1 Error output:
'android' is not recognized as an internal or external command,
operable program or batch file.]
  stderr: '\'android\' is not recognized as an internal or external 
command,\r\noperable program or batch file.\r\n',
  code: 1 }
```

Not sure how to downgrade (possible?) if not I would have tested that.



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-16 Thread alsorokin 
Github user alsorokin commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
I'll create a test job that uses this code when our slaves are back up 
(some Microsoft infra permutations are underway ATM).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-16 Thread filmaj 
Github user filmaj commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
Oh yes, and ping @alsorokin - not sure if the changes to the android CLI 
scripts affect the CI in any way? But in any case, probably worth getting your 
eyes on this change too :)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-16 Thread filmaj 
Github user filmaj commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
@dpogue thanks for checking! So I think `android_sdk_version` failing in 
this case is kind of expected, as that script, historically, did not do any 
environment checking, and I tried keeping the same behaviour.

The other commands that are part of `Api.js`, like `build` and `clean` and 
`run`, [always call through to 
`check_reqs`](https://github.com/apache/cordova-android/blob/master/bin/templates/cordova/Api.js#L344-L346).
 Part of that functionality in this pull request now [automatically adds the 
new SDK tools to your 
`PATH`](https://github.com/filmaj/cordova-android/blob/CB-12546/bin/templates/cordova/lib/check_reqs.js#L295-L297)
 if they are missing. That's why the `build` command worked for you, but the 
`android_sdk_version` command did not.

We could tweak the behaviour of `android_sdk_version` so that it calls 
through to `check_reqs` as well? This change would fix that command failing in 
your case, and provide more meaningful errors for people who don't have their 
environments configured properly (i.e. will check for missing `ANDROID_HOME`, 
will munge `PATH` based on what kinds of Android tooling it will find, etc.). 
What do you think?

The deeper question that rises out of this is: is `android_sdk_version` 
useful? Is it worth keeping around? It is not part of the platform API and is 
still hanging around due to history more than anything else.

Let me know what y'all think.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-15 Thread dpogue 
Github user dpogue commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
### Linux with Android SDK 25.2.2
✅ npm test
✅ ./bin/check_reqs
✅ ./bin/android_sdk_version
✅ ./bin/create
✅ ./cordova/check_reqs
✅ ./cordova/android_sdk_version
✅ ./cordova/build
✅ ./cordova/clean
(No emulator images installed, so I didn't try that step)

### Linux with Android SDK 25.3.1
✅ npm test
✅ ./bin/check_reqs
❌ ./bin/android_sdk_version
```
{ Error: sdkmanager: Command failed with exit code ENOENT
at ChildProcess.whenDone 
(/mnt/users/dpogue/Coding/cordova-android/node_modules/cordova-common/src/superspawn.js:169:23)
at emitOne (events.js:96:13)
at ChildProcess.emit (events.js:191:7)
at Process.ChildProcess._handle.onexit 
(internal/child_process.js:213:12)
at onErrorNT (internal/child_process.js:367:16)
at _combinedTickCallback (internal/process/next_tick.js:80:11)
at process._tickCallback (internal/process/next_tick.js:104:9) code: 
'ENOENT' }
```
✅ ./bin/create
✅ ./cordova/check_reqs
❌ ./cordova/android_sdk_version
```
{ Error: sdkmanager: Command failed with exit code ENOENT
at ChildProcess.whenDone 
(/mnt/users/dpogue/Coding/cordova-android/node_modules/cordova-common/src/superspawn.js:169:23)
at emitOne (events.js:96:13)
at ChildProcess.emit (events.js:191:7)
at Process.ChildProcess._handle.onexit 
(internal/child_process.js:213:12)
at onErrorNT (internal/child_process.js:367:16)
at _combinedTickCallback (internal/process/next_tick.js:80:11)
at process._tickCallback (internal/process/next_tick.js:104:9) code: 
'ENOENT' }
```
✅ ./cordova/build
✅ ./cordova/clean

It looks like the two failures are due to not having the `sdkmanager` tool 
on my path, but it should be able to figure it out based on `ANDROID_HOME`?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #369: Updated CLI scripts to support Android SDK Tools...

2017-03-15 Thread codecov-io 
Github user codecov-io commented on the issue:

https://github.com/apache/cordova-android/pull/369
  
# 
[Codecov](https://codecov.io/gh/apache/cordova-android/pull/369?src=pr=h1) 
Report
> Merging 
[#369](https://codecov.io/gh/apache/cordova-android/pull/369?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/cordova-android/commit/84de9ee0da0551cb16896953777f78d2a6d5e58d?src=pr=desc)
 will **increase** coverage by `7.84%`.
> The diff coverage is `71%`.


```diff
@@Coverage Diff @@
##   master #369  +/-   ##
==
+ Coverage   35.58%   43.42%   +7.84% 
==
  Files  12   14   +2 
  Lines1037 1347 +310 
  Branches  173  247  +74 
==
+ Hits  369  585 +216 
- Misses668  762  +94
```


| [Impacted 
Files](https://codecov.io/gh/apache/cordova-android/pull/369?src=pr=tree) | 
Coverage Δ | |
|---|---|---|
| 
[bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/compare/84de9ee0da0551cb16896953777f78d2a6d5e58d...389d81b972cc272b4911f0140891a5fb6c740abd?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz)
 | `48.25% <55.42%> (ø)` | |
| 
[bin/templates/cordova/lib/android\_sdk.js](https://codecov.io/gh/apache/cordova-android/compare/84de9ee0da0551cb16896953777f78d2a6d5e58d...389d81b972cc272b4911f0140891a5fb6c740abd?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9hbmRyb2lkX3Nkay5qcw==)
 | `81.81% <81.81%> (ø)` | |
| 
[bin/templates/cordova/lib/emulator.js](https://codecov.io/gh/apache/cordova-android/compare/84de9ee0da0551cb16896953777f78d2a6d5e58d...389d81b972cc272b4911f0140891a5fb6c740abd?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9lbXVsYXRvci5qcw==)
 | `38.51% <82.25%> (+24.62%)` | :white_check_mark: |

--

[Continue to review full report at 
Codecov](https://codecov.io/gh/apache/cordova-android/pull/369?src=pr=continue).
> **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute  (impact)`, `ø = not affected`, `? = missing 
data`
> Powered by 
[Codecov](https://codecov.io/gh/apache/cordova-android/pull/369?src=pr=footer).
 Last update 
[84de9ee...389d81b](https://codecov.io/gh/apache/cordova-android/compare/84de9ee0da0551cb16896953777f78d2a6d5e58d...389d81b972cc272b4911f0140891a5fb6c740abd?el=footer=pr=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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org