Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(deps): Switch from adbkit to @devicefarmer/adbkit npm package #2039

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

rpl
Copy link
Member

@rpl rpl commented Oct 2, 2020

Fixes #2025

Some notes from some additional checks I did:

  • The developer working on the replacement package @devicefarmer/adbkit is also part of the organization under which the adbkit package was developed, and the new @devicefarmer/adbkit seems to be actively maintained as discussed in the issue linked above.

  • I also looked to the diff between the 3 adbkit/adbkit-* packages we are currently using and the 3 related package under the new devicefarmer github organization (both in their original coffeescript sources and the resulting transpiled js code) and the changes are minimal, matching what we would expect from a patch release, and nothing that should change the behavior of the code that is already using the adbkit package in web-ext.

  • To be totally fair, there is some small changes around the adb connection object that at a first glance may trigger some slightly different behaviors in case of connection errors, but I do have tested web-ext run on a real android device and tried the behavior on unexpected disconnection of the device and the behavior seems to be the same (there is some fixes that we should apply around that, but it isn't a blocker for this pull request, I'll file it as a follow up).

@rpl rpl requested a review from Rob--W October 2, 2020 15:53
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 162f625 on rpl:fix/update-adbkit-dependency into 5492a0b on mozilla:master.

@rpl rpl merged commit 411a366 into mozilla:master Oct 5, 2020
@Standard8
Copy link
Member

@Rob--W would it be possible to get a new release with this in, to help make it easier to get the node-forge update?

@rpl
Copy link
Member Author

rpl commented Oct 5, 2020

@Rob--W would it be possible to get a new release with this in, to help make it easier to get the node-forge update?

@Standard8 I'm going to release a new minor version of web-ext asap (I should be able to release it today).

@rpl
Copy link
Member Author

rpl commented Oct 5, 2020

@Standard8 this fix has been just released on npm as part of web-ext v5.2.0.

@Standard8
Copy link
Member

Thank you!

sonalprabhu pushed a commit to sonalprabhu/web-ext that referenced this pull request Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace adbkit dependency
4 participants