cordova-cli icon indicating copy to clipboard operation
cordova-cli copied to clipboard

feat!: remove telemetry feature

Open shajz opened this issue 2 years ago • 2 comments

Platforms affected

n/a

Motivation and Context

Closes https://github.com/apache/cordova-cli/issues/625.

Telemetry relies on the insight package, which is unmaintained and relies on the request package which has a CVE https://github.com/advisories/GHSA-p8p7-x288-28g6

This is basically the same PR as https://github.com/apache/cordova-cli/pull/614 but it's now following this discussion with the backing of @breautek : https://github.com/apache/cordova-cli/pull/631#issuecomment-1856101038

Description

Remove the telemetry feature entirely

Testing

Ran npm run test successfully

Checklist

  • [x] I've run the tests to see all new and existing tests pass
  • [x] I added automated test coverage as appropriate for this change
  • [x] 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)
  • [x] I've updated the documentation if necessary

shajz avatar Dec 14 '23 16:12 shajz

Haha @breautek, which PR do we keep? https://github.com/apache/cordova-cli/pull/632

shajz avatar Dec 14 '23 16:12 shajz

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.53%. Comparing base (bf1f561) to head (46f9941).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #633      +/-   ##
==========================================
- Coverage   70.68%   65.53%   -5.16%     
==========================================
  Files           4        3       -1     
  Lines         307      235      -72     
==========================================
- Hits          217      154      -63     
+ Misses         90       81       -9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Dec 14 '23 17:12 codecov-commenter

Hi @shajz, thanks for the PR! This one seems slightly more complete than #632 in that it also cleans up the documentation.

I'd really like to get this merged, if you have a chance to rebase it to resolve the conflicts on package.json. If you're busy, I can try to cherry-pick it onto a new branch next week and open a new PR retaining credit to you for the commit.

dpogue avatar Jun 17 '24 16:06 dpogue

Hi @dpogue, I've rebased on master and fixed the conflicts on the package.json and package-lock.json files :)

I'll also mark the PR as ready for review, thanks for the interest in this PR!

shajz avatar Jun 17 '24 17:06 shajz

Thank you @shajz! 🎉 Apologies for letting this sit open for so long.

dpogue avatar Jun 17 '24 17:06 dpogue