You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2020/10/20 10:27:50 UTC
[GitHub] [cordova-android] raphinesse opened a new pull request #1103: refactor: do not kill adb on UNIX-like systems
raphinesse opened a new pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
Closes #1098
### Description
<!-- Describe your changes in detail -->
This basically reverts 66fa12a.
### Testing
<!-- Please describe in detail how you tested your changes. -->
Unit tests pass & works for me on Linux
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io commented on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-712837293
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=h1) Report
> Merging [#1103](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/335b0f2575bdcb432583132f17b5f141c64e7c0b?el=desc) will **increase** coverage by `0.18%`.
> The diff coverage is `60.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1103 +/- ##
==========================================
+ Coverage 69.71% 69.89% +0.18%
==========================================
Files 20 20
Lines 1806 1784 -22
==========================================
- Hits 1259 1247 -12
+ Misses 547 537 -10
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/build.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZC5qcw==) | `33.33% <0.00%> (+2.22%)` | :arrow_up: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `57.54% <0.00%> (-1.42%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1103?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/1103?src=pr&el=footer). Last update [335b0f2...d621b6d](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] NiklasMerz commented on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-713318941
I think my linux system is set up weirdly. I sometimes get an error when running the adb command that I am using a different version and it restarts. But looking at the log outputs in this PR this is somthing different. I can't really tell if this is still useful :man_shrugging: 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-713413052
> I sometimes get an error when running the adb command that I am using a different version and it restarts.
Which adb command? It sounds like you are talking about app installation. That has not been touched here.
> But looking at the log outputs in this PR this is somthing different. I can't really tell if this is still useful
I don't think I can follow you. Did you observe any change in behavior with the change from this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] NiklasMerz commented on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-713447491
I did not test this PR locally, yet. Nevermind I was just thinking loud because I always had strange adb behaviour but that is most likely not caused by Cordova.
From the issue:
> The problem this killall is supposed to solve might not even exist anymore. I remember adb losing devices a few years back but it hasn't happened to me in a long time. It would be great to know if someone still encounters these problems today on a regular basis. If not, we could simply put an end to all that killing 😉
I often have trouble with lost connections to my real device on a Linux machine. I feel this is problem with my setup and it's hard to reproduce properly.
This is probably a good change but I am unsure about how to test this👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io edited a comment on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-712837293
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=h1) Report
> Merging [#1103](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/335b0f2575bdcb432583132f17b5f141c64e7c0b?el=desc) will **increase** coverage by `0.29%`.
> The diff coverage is `60.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1103 +/- ##
==========================================
+ Coverage 69.71% 70.01% +0.29%
==========================================
Files 20 20
Lines 1806 1784 -22
==========================================
- Hits 1259 1249 -10
+ Misses 547 535 -12
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/build.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZC5qcw==) | `33.33% <0.00%> (+2.22%)` | :arrow_up: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `58.49% <0.00%> (-0.48%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1103?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/1103?src=pr&el=footer). Last update [335b0f2...d621b6d](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io edited a comment on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-712837293
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=h1) Report
> Merging [#1103](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/335b0f2575bdcb432583132f17b5f141c64e7c0b?el=desc) will **increase** coverage by `0.35%`.
> The diff coverage is `60.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1103 +/- ##
==========================================
+ Coverage 69.71% 70.06% +0.35%
==========================================
Files 20 20
Lines 1806 1784 -22
==========================================
- Hits 1259 1250 -9
+ Misses 547 534 -13
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/build.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZC5qcw==) | `33.33% <0.00%> (+2.22%)` | :arrow_up: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/1103/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `100.00% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1103?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/1103?src=pr&el=footer). Last update [335b0f2...d621b6d](https://codecov.io/gh/apache/cordova-android/pull/1103?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse merged pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
raphinesse merged pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-713590716
If nobody objects I will merge this into master soon. IMO this is more a fix than anything else.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-713553274
In my experience, at least on Linux, adb tools is a finicky program that barely works. Often I have to unplug the USB device and replug it in and ADB will start magically working again. I'm not sure if this is a problem we can solve on the Cordova side. Obviously https://github.com/apache/cordova-android/commit/66fa12a091c44d062ce4423e9e413e647895c466 was probably a good attempt but I don't think it solved the issue.
In the long term, I envision that we abandoned adb for most, if not all tasks anyway, and use the [bundletool](https://github.com/google/bundletool) instead. If we're lucky it will fix all ADB related issues, and give us a whole brand new set of challenges :smile:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1103: refactor: do not kill adb on UNIX-like systems
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1103:
URL: https://github.com/apache/cordova-android/pull/1103#issuecomment-713541959
> This is probably a good change but I am unsure about how to test this
Yeah, it is hard to test in how far adb stability is still an issue. I would suggest we remove the current workaround and in case people report frequent hangs, we add a timeout to short-running adb commands.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org