erisu opened a new pull request, #614: URL: https://github.com/apache/cordova-cli/pull/614
<!-- 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 n/a ### 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. --> 1. Appear that the insight package may no longer being updated or maintained. It is not deprecated or archived as of yet. https://github.com/yeoman/insight/issues/83 - Appears to not be making efforts to update the package. * Appears that yeoman may also no longer use this package as well. 1. Sub-dependencies that requires upgrading was already converted to ESM. Both Cordova and insight is not prepared for ESM. * One of those Sub-dependencies contains the macOS name lookup values. Insight is stuck on a version that can only lookup to macOS 12. * Some people reported CLI crashing when running on macOS13. Caused by the macOS name lookup package. * I am unable to reproduce this issue anymore. 1. One person showed concern for the version of the UUID package that is used in one of the sub-dependencies within insight but I find it more of a warning and not an issue. > Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details. I don't know if anyone actively looks at the telemetry data. Usually the data is useful for making arguments in what we should be supported or dropped in terms of Cordova tooling. Due to the activity of PMC members, we usually focus only on the last major release. We do no usually backport fixes or features. Last time I used the data was for making an argument point in what Cordova Docs versions data should be dropped, but in the end it seems everyone wanted to keep and support all version docs, even though we do not support older versions. It is easier to remove the current telemetry logic and package then trying to replace the package and refactor. We can always read telemetry in the future with an alternative package, and implement it in a future minor release. But the motivation is for a quick release of CLI 12. We either drop it to resolve the points above, if all are valid or not, or we push the release as is, with insight, and pray that those who had crashing CLI was just a small % and cause by some unusual use case that was resolved. If we add back telemetry, we should investigate alternative solutions, for example: [@analytics/google-analytics](https://www.npmjs.com/package/@analytics/google-analytics) We would need to maybe write our own wrapper class, a permission request to see if users want to enable or disable analytics and storing value. ### Description <!-- Describe your changes in detail --> - remove telemetry - remove insight ### Testing <!-- Please describe in detail how you tested your changes. --> `npm t` ### 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)`) - [ ] 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 -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
