search

yeoman / yo

CLI tool for running Yeoman generators
http://yeoman.io
BSD 2-Clause "Simplified" License
3.8k stars 396 forks source link

Remove `insight` module for now #771

Closed mischah closed 1 year ago

mischah commented 1 year ago

This is a quick solution to fix #753

Purpose of this pull request?

What changes did you make?

Completely removed the insight module for now. So user tracking will be gone once this version is published. I’ll be happy to bring back insight when the related work is done. Quoting from #753 to give context:

It’s not as easy as I thought 🙈 Because we have to update sindresorhus/os-name to v5.0.1 in yeoman/insight.

BUT: os-name is a pure ESM since 5.0.0. which means we have to change the module system in insight as well as described over here: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c

… which means we have to refactor yeoman/yo as well to make this work 😔

Is there anything you'd like reviewers to focus on?

@SBoudrias Do you think this is valid approach to get this bugfix out as soon as possible?

@mshima I saw that there were a lot of dependency updates with major version in the main branch since the last release. Are we sure, that we don’t introduce a breaking change? I would love to publish this fix as a patch release. I hope we can trust the tests so that yo still runs on Node 12.x 😬

SBoudrias commented 1 year ago

LGTM, I don't think anyone of us is still looking at the insight data. So shouldn't have any big impact.