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
chore: Replace npmlog
dependency in order to support Node 10+
#1392
Conversation
To support node >= 10 instead of >= 12.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a break change for logging behavior, as npmlog
handles npm's loglevel
config, as well as --silent
and --verbose
flags.
I will need to review it later, so please hold on with merging this.
Does it actually? When I run this locally on master, I see logs no matter what npm loglevel I use, and no matter what flags I use: ➜ sentry-cli git:(master) npm config set loglevel warn
➜ sentry-cli git:(master) yarn
yarn install v1.22.19
[1/5] 🔍 Validating package.json...
[2/5] 🔍 Resolving packages...
success Already up-to-date.
$ node ./scripts/install.js
info sentry-cli Using cached binary: ~/.npm/sentry-cli/d8b881-sentry-cli-Darwin-universal
✨ Done in 0.72s.
➜ sentry-cli git:(master) node ./scripts/install.js --silent
info sentry-cli Using cached binary: ~/.npm/sentry-cli/d8b881-sentry-cli-Darwin-universal But also TBH not quite sure how that should work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's settle on bringing support down to v10
and dropping npmlog
.
It's not worth to spend time on this, blocking other people, while solution is rather non-intrusive enough :)
npmlog
dependency in order to support Node 10+npmlog
dependency in order to support Node 10+
This PR replaces the npmlog dependency with a super simple logger. We actually only use very limited stuff from npmlog, so we can drop this dependency, which, as far as I can tell, is the only thing blocking us from supporting node 10.
This does loose some coloring in the generated logs, but IMHO that is not that important - if we'd really want to, we can still bring this back with something like
chalk
.I also downgraded eslint to 7.x, as then we can run the whole repo on node 10, which makes it easier to verify everything works there.
Closes #1391