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/06/06 16:28:41 UTC
[GitHub] [cordova-plugin-network-information] PieterVanPoyer opened a new pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
PieterVanPoyer opened a new pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114
<!--
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
### 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. -->
Fixes https://github.com/apache/cordova-plugin-network-information/issues/110 .
### Description
<!-- Describe your changes in detail -->
The JSONObject of Android does not have an equals implementation. So the equals between previous networkinfo and new networkinfo state always returned false.
I've explicitly tested all properties of the JSONObject for equality.
### Testing
<!-- Please describe in detail how you tested your changes. -->
I did test it on an Android Nexus 5X Emulator.
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
- [ ] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
I didn't prefix the commit, but I did prefix the 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-plugin-network-information] breautek commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-678329401
Sorry for the lack of timely response, I think it got lost in the bucket of emails.
> Can we speed up the release process in some way? If yes, how?
I personally would like to obtain npm access so that I can prepare releases, but I'm not sure if I'm equipped to do that. Ie. I have no mac hardware, so I'm not sure if that limits my ability to do releases or not. It would definitely limit my ability to conduct local native tests for ios code, for repos that have them.
> Working on the plugin was not that smooth for me, I did copy paste the Java code from a test project back to the plugin project. Is there a better workflow, for developing or contributing to plugins?
This is pretty much what I do, but because I don't really delve into native code that much... I believe if you use the `platform add` or `plugin add` with the `--link` flag, it will create symbolic links back to the repo.
e.g: `cordova plugin add file:../cordova-plugin-network-information --link`
> Is it possible to open an issue or a milestone, or something to discuss the future requirements of the plugin (v4). Something like a whitepaper. I did made some suggestions in this comment #110 (comment) . You did already do some suggestions in next comment: #91 (comment)
We generally use github projects, or sometimes a meta ticket like https://github.com/apache/cordova/issues/142 for example.
Other ways to get more involved is by joining our [slack](http://slack.cordova.io/), and subscribing to the [dev mailing list](https://cordova.apache.org/contact/)
And of course, ideas for improving workflows is always welcome.
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer edited a comment on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer edited a comment on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-646252009
> It would also be nice if you could bring all the else/else if statement of `getType` to the same line as the bracket.
I am not entirely sure about your remark. But I did improve it a little bit.
Or do you mean changing this
```
} else if (type.startsWith(CDMA) ||
type.equals(UMTS) ||
type.equals(ONEXRTT) ||
type.equals(EHRPD) ||
type.equals(HSUPA) ||
type.equals(HSDPA) ||
type.equals(HSPA) ||
type.equals(THREE_G)) {
return TYPE_3G;
}
```
to something like this?
```
} else if (type.startsWith(CDMA) || type.equals(UMTS) || type.equals(ONEXRTT) || type.equals(EHRPD) || type.equals(HSUPA) || type.equals(HSDPA) || type.equals(HSPA) || type.equals(THREE_G)) {
return TYPE_3G;
}
```
----------------------------------------------------------------
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-plugin-network-information] breautek commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r436283641
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
As long as this doesn't affect the `PluginResult` responses, then I don't have any objection. But do you think it's necessary to be as a part of this bugfix PR, or should that change be a separate refactor 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-plugin-network-information] PieterVanPoyer commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-650382752
Hey @timbru31 or @erisu
I do not like to disturb you guys, but does anyone have time to review/approve this PR?
I am sometimes available to discuss this PR.
Kind regards
Pieter
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-658661248
Hey
I did enjoy contributing a little.
Your enthusiasm is infectious.
I got a few questions
- Can we speed up the release process in some way? If yes, how?
- Working on the plugin was not that smooth for me, I did copy paste the Java code from a test project to the plugin project. Is there a better workflow, for developing or contributing to plugins?
- Is it possible to open an issue or a milestone, or something to discuss the future requirements of the plugin (v4). Something like a whitepaper. I did made some suggestions in this comment https://github.com/apache/cordova-plugin-network-information/issues/110#issuecomment-639588835 . You did already do some suggestions in next comment: https://github.com/apache/cordova-plugin-network-information/issues/91#issuecomment-580906459
Kind regards
Pieter
----------------------------------------------------------------
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-plugin-network-information] breautek commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r436283641
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
As long as this doesn't affect the `PluginResult` responses, then I don't have any objection. But do you think it's necessary to be as a part of a bugfix PR, or should that change be a separate refactor 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-plugin-network-information] PieterVanPoyer edited a comment on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer edited a comment on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-658661248
Hey @breautek
I did enjoy contributing a little.
Your enthusiasm is infectious.
I got a few questions
- Can we speed up the release process in some way? If yes, how?
- Working on the plugin was not that smooth for me, I did copy paste the Java code from a test project back to the plugin project. Is there a better workflow, for developing or contributing to plugins?
- Is it possible to open an issue or a milestone, or something to discuss the future requirements of the plugin (v4). Something like a whitepaper. I did made some suggestions in this comment https://github.com/apache/cordova-plugin-network-information/issues/110#issuecomment-639588835 . You did already do some suggestions in next comment: https://github.com/apache/cordova-plugin-network-information/issues/91#issuecomment-580906459
Kind regards
Pieter
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r442434500
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +262,36 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
-
- LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
- LOG.d(LOG_TAG, "wifi : " + WIFI);
- if (type.equals(WIFI)) {
- return TYPE_WIFI;
- }
- else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
- return TYPE_ETHERNET;
- }
- else if (type.equals(MOBILE) || type.equals(CELLULAR)) {
- type = info.getSubtypeName().toLowerCase(Locale.US);
- if (type.equals(GSM) ||
+ String type = info.getTypeName().toLowerCase(Locale.US);
+
+ LOG.d(LOG_TAG, "toLower : " + type);
+ if (type.equals(WIFI)) {
+ return TYPE_WIFI;
+ } else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
Review comment:
toLowerCase() is still present on this line. But I rather leave this one like it is.
I don't like to change something that I am unable to test easy.
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-646252009
> It would also be nice if you could bring all the else/else if statement of `getType` to the same line as the bracket.
I am not entirely sure about your remark. But I did improve it a little bit.
----------------------------------------------------------------
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-plugin-network-information] davidpadych commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
davidpadych commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-655430888
Hi,
when will the new version be released? merged with this PR .. thank you
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-646121629
Hey @timbru31 or @erisu
I do not like to disturb you guys, but does anyone have time to review this PR?
I am always available to discuss this PR.
Kind regards
Pieter
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r436345937
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
I did take a second look to the code.
The pluginResult is just the "type" of network.
So this made my reflect a little bit about the "extraInfo" state.
```
JSONObject connectionInfo = new JSONObject();
try {
connectionInfo.put("type", type);
connectionInfo.put("extraInfo", extraInfo);
} catch (JSONException e) {
LOG.d(LOG_TAG, e.getLocalizedMessage());
}
```
If I read the code the "extraInfo" is not used at all, it is stored in the JsonObject, but is not used or read anywhere else in the NetworkManager class.
This remark is just about the boyscout rule ( Leave your code better than you found it ).
Maybe a next developer is happy that someone left it cleaner.
So my suggestions are:
- remove the JSONObject, just use a String for type as state.
- remove the "extraInfo" state.
I leave it over to your decision if it should be a seperate PR, or not.
But I am willing to make these changes in this or another PR. (It's milestone 3, so there is time?)
----------------------------------------------------------------
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-plugin-network-information] breautek commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-653087429
It looks like all the requested changes from @timbru31 have been made.
I'm going to say a last call for reviews. If there are no responses by Tuesday, July 7th, I'll merge this in.
----------------------------------------------------------------
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-plugin-network-information] breautek commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-640938718
Do let me know when it's ready for second review.
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r442425460
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +262,37 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
-
- LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
- LOG.d(LOG_TAG, "wifi : " + WIFI);
- if (type.equals(WIFI)) {
- return TYPE_WIFI;
- }
- else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
- return TYPE_ETHERNET;
- }
- else if (type.equals(MOBILE) || type.equals(CELLULAR)) {
- type = info.getSubtypeName().toLowerCase(Locale.US);
- if (type.equals(GSM) ||
+ String type = info.getTypeName().toLowerCase(Locale.US);
+
+ LOG.d(LOG_TAG, "toLower : " + type);
+ LOG.d(LOG_TAG, "wifi : " + WIFI);
Review comment:
Why do we even log the WIFI time after time. It just the constant wifi.
I think this can just be removed.
----------------------------------------------------------------
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-plugin-network-information] breautek commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-655509380
> Hi,
> when will the new version be released? merged with this PR .. thank you
Can't really provide any ETAs on releases.
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r436281929
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
Maybe we should just get rid of the JSONObject in this plugin.
We could refactor it to a seperate (VO-class) with two fields: 'type' and 'extraInfo'.
This class could have a correct equals and hashcode function.
This could make the try - catch constructions with JSONException obsolete on a lof of places.
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r436791107
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
Done.
I am not 100% with the naming of the method: getTypeOfNetworkFallbackToTypeNoneIfNotConnected , but it is okay.
My tests on the emulator were successfull. I am planning on doing some tests on my mobile phone.
There is still more unused code in the plugin in my opinion, but that's for another iteration.
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
Done.
I am not 100% satisfied with the naming of the method: getTypeOfNetworkFallbackToTypeNoneIfNotConnected , but it is okay.
My tests on the emulator were successfull. I am planning on doing some tests on my mobile phone.
There is still more unused code in the plugin in my opinion, but that's for another iteration.
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
Tests on the mobile phone were successfull. Seems to work.
----------------------------------------------------------------
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-plugin-network-information] breautek commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r442427469
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +262,37 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
-
- LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
- LOG.d(LOG_TAG, "wifi : " + WIFI);
- if (type.equals(WIFI)) {
- return TYPE_WIFI;
- }
- else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
- return TYPE_ETHERNET;
- }
- else if (type.equals(MOBILE) || type.equals(CELLULAR)) {
- type = info.getSubtypeName().toLowerCase(Locale.US);
- if (type.equals(GSM) ||
+ String type = info.getTypeName().toLowerCase(Locale.US);
+
+ LOG.d(LOG_TAG, "toLower : " + type);
+ LOG.d(LOG_TAG, "wifi : " + WIFI);
Review comment:
Agreed.
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r442426866
##########
File path: src/android/NetworkManager.java
##########
@@ -167,16 +161,11 @@ public void onReceive(Context context, Intent intent) {
updateConnectionInfo(sockMan.getActiveNetworkInfo());
}
- String connectionType = null;
- if(NetworkManager.this.lastInfo == null) {
+ String connectionType;
+ if(NetworkManager.this.lastTypeOfNetwork == null) {
Review comment:
done
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-640987774
Ready for 2nd review.
----------------------------------------------------------------
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-plugin-network-information] breautek commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r436372553
##########
File path: src/android/NetworkManager.java
##########
@@ -230,9 +230,44 @@ private void updateConnectionInfo(NetworkInfo info) {
sendUpdate(connectionType);
lastInfo = thisInfo;
+ } else {
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
}
}
+ private boolean connectionInfoDiffersFromLastInfo(JSONObject thisInfo) {
+ // JSONObject.equals does not work, so the equals is done explicit for every key in the object
+ if(lastInfo == null) {
+ return thisInfo != null;
+ }
+
+ if(thisInfo == null) {
+ return true;
+ }
+
+ if (valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "type")) {
+ return true;
+ }
+
+ return valueInJSONObjectDiffersForKey(thisInfo , lastInfo, "extraInfo");
+ }
+
+ private boolean valueInJSONObjectDiffersForKey(JSONObject firstObject, JSONObject secondObject, String key) {
Review comment:
+1 on removing `extraInfo` is not used at all.
+1 on converting `JSONObject` to simply use strings for state.
In regards whether to do it as a bugfix PR, I guess it makes sense to keep the refactor together if the refactor is going to heavily touch the code you added in this PR, which I think is the case since you will be removing usages of `JSONObject`.
----------------------------------------------------------------
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-plugin-network-information] breautek edited a comment on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek edited a comment on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-678329401
Sorry for the lack of timely response, I think it got lost in the bucket of emails.
> Can we speed up the release process in some way? If yes, how?
Cordova is entirely ran by volunteers, so generally speaking we're at the mercy of a volunteer, who happens to have release permissions to spend their time preparing the release, which is in itself a lengthy process.
I personally would like to obtain npm access so that I can prepare releases, but I'm not sure if I'm equipped to do that. Ie. I have no mac hardware, so I'm not sure if that limits my ability to do releases or not. It would definitely limit my ability to conduct local native tests for ios code, for repos that have them.
> Working on the plugin was not that smooth for me, I did copy paste the Java code from a test project back to the plugin project. Is there a better workflow, for developing or contributing to plugins?
This is pretty much what I do, but because I don't really delve into native code that much... I believe if you use the `platform add` or `plugin add` with the `--link` flag, it will create symbolic links back to the repo.
e.g: `cordova plugin add file:../cordova-plugin-network-information --link`
> Is it possible to open an issue or a milestone, or something to discuss the future requirements of the plugin (v4). Something like a whitepaper. I did made some suggestions in this comment #110 (comment) . You did already do some suggestions in next comment: #91 (comment)
We generally use github projects, or sometimes a meta ticket like https://github.com/apache/cordova/issues/142 for example.
Other ways to get more involved is by joining our [slack](http://slack.cordova.io/), and subscribing to the [dev mailing list](https://cordova.apache.org/contact/)
And of course, ideas for improving workflows is always welcome.
----------------------------------------------------------------
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-plugin-network-information] PieterVanPoyer commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r442427378
##########
File path: src/android/NetworkManager.java
##########
@@ -218,30 +207,25 @@ private void unregisterReceiver() {
private void updateConnectionInfo(NetworkInfo info) {
// send update to javascript "navigator.network.connection"
// Jellybean sends its own info
- JSONObject thisInfo = this.getConnectionInfo(info);
- if(!thisInfo.equals(lastInfo))
+ String currentNetworkType = this.getTypeOfNetworkFallbackToTypeNoneIfNotConnected(info);
+ if(currentNetworkType.equals(this.lastTypeOfNetwork))
{
Review comment:
done. In the other parts of the NetworkManager class, there are also strange indentations, but I rather leave them untouched.
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +263,41 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
+ String type = info.getTypeName().toLowerCase(Locale.US);
Review comment:
Seems like it never returns null.
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +263,41 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
+ String type = info.getTypeName().toLowerCase(Locale.US);
- LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
- LOG.d(LOG_TAG, "wifi : " + WIFI);
- if (type.equals(WIFI)) {
- return TYPE_WIFI;
- }
- else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
- return TYPE_ETHERNET;
- }
- else if (type.equals(MOBILE) || type.equals(CELLULAR)) {
- type = info.getSubtypeName().toLowerCase(Locale.US);
- if (type.equals(GSM) ||
+ LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
Review comment:
I'vre removed the lowercase.
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +263,41 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
+ String type = info.getTypeName().toLowerCase(Locale.US);
- LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
- LOG.d(LOG_TAG, "wifi : " + WIFI);
- if (type.equals(WIFI)) {
- return TYPE_WIFI;
- }
- else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
- return TYPE_ETHERNET;
- }
- else if (type.equals(MOBILE) || type.equals(CELLULAR)) {
- type = info.getSubtypeName().toLowerCase(Locale.US);
- if (type.equals(GSM) ||
+ LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
Review comment:
I did remove the lowercase.
----------------------------------------------------------------
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-plugin-network-information] timbru31 commented on a change in pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#discussion_r442336135
##########
File path: src/android/NetworkManager.java
##########
@@ -167,16 +161,11 @@ public void onReceive(Context context, Intent intent) {
updateConnectionInfo(sockMan.getActiveNetworkInfo());
}
- String connectionType = null;
- if(NetworkManager.this.lastInfo == null) {
+ String connectionType;
+ if(NetworkManager.this.lastTypeOfNetwork == null) {
Review comment:
```suggestion
if (NetworkManager.this.lastTypeOfNetwork == null) {
```
##########
File path: src/android/NetworkManager.java
##########
@@ -218,30 +207,25 @@ private void unregisterReceiver() {
private void updateConnectionInfo(NetworkInfo info) {
// send update to javascript "navigator.network.connection"
// Jellybean sends its own info
- JSONObject thisInfo = this.getConnectionInfo(info);
- if(!thisInfo.equals(lastInfo))
+ String currentNetworkType = this.getTypeOfNetworkFallbackToTypeNoneIfNotConnected(info);
+ if(currentNetworkType.equals(this.lastTypeOfNetwork))
{
Review comment:
```suggestion
if (currentNetworkType.equals(this.lastTypeOfNetwork)) {
```
##########
File path: src/android/NetworkManager.java
##########
@@ -218,30 +207,25 @@ private void unregisterReceiver() {
private void updateConnectionInfo(NetworkInfo info) {
// send update to javascript "navigator.network.connection"
// Jellybean sends its own info
- JSONObject thisInfo = this.getConnectionInfo(info);
- if(!thisInfo.equals(lastInfo))
+ String currentNetworkType = this.getTypeOfNetworkFallbackToTypeNoneIfNotConnected(info);
+ if(currentNetworkType.equals(this.lastTypeOfNetwork))
{
- String connectionType = "";
- try {
- connectionType = thisInfo.get("type").toString();
- } catch (JSONException e) {
- LOG.d(LOG_TAG, e.getLocalizedMessage());
- }
-
- sendUpdate(connectionType);
- lastInfo = thisInfo;
+ LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the javascript side.");
Review comment:
```suggestion
LOG.d(LOG_TAG, "Networkinfo state didn't change, there is no event propagated to the JavaScript side.");
```
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +263,41 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
+ String type = info.getTypeName().toLowerCase(Locale.US);
- LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
- LOG.d(LOG_TAG, "wifi : " + WIFI);
- if (type.equals(WIFI)) {
- return TYPE_WIFI;
- }
- else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
- return TYPE_ETHERNET;
- }
- else if (type.equals(MOBILE) || type.equals(CELLULAR)) {
- type = info.getSubtypeName().toLowerCase(Locale.US);
- if (type.equals(GSM) ||
+ LOG.d(LOG_TAG, "toLower : " + type.toLowerCase());
Review comment:
The `toLowerCase()` seems redundant.
##########
File path: src/android/NetworkManager.java
##########
@@ -289,46 +263,41 @@ private void sendUpdate(String type) {
* @return the type of mobile network we are on
*/
private String getType(NetworkInfo info) {
- if (info != null) {
- String type = info.getTypeName().toLowerCase(Locale.US);
+ String type = info.getTypeName().toLowerCase(Locale.US);
Review comment:
Can `getTypeName()` ever return null? We've either need to guard this or reverse all `.equals` checks.
----------------------------------------------------------------
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-plugin-network-information] breautek merged pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek merged pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114
----------------------------------------------------------------
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-plugin-network-information] breautek commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-655511437
There was no responses, so I'm merging in.
You can also ignore the TotalPave review, that was me on the wrong account.
Thank you @PieterVanPoyer for your time and effort.
----------------------------------------------------------------
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-plugin-network-information] breautek commented on pull request #114: Android - Fixes https://github.com/apache/cordova-plugin-network-information#110
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #114:
URL: https://github.com/apache/cordova-plugin-network-information/pull/114#issuecomment-640088345
Comments on the original still applies: https://github.com/apache/cordova-plugin-network-information/pull/111#issuecomment-640073128
With the important note of:
> This looks good to me. While this is fixing a bug, it is changing behaviour (which is to stop event firing on resume, if there was no change in state), which is a behaviour that some people may be dependent on. Because of this, I think we need to treat this as a breaking change.
----------------------------------------------------------------
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