Switch from console.log to npmlog #1786

mcdado · 2016-11-03T22:04:19Z

npmlog has the advantage of using the same log level configuration of npm.

Running npm install gulp-sass --no-progress --silent I found out that this package is particularly noisy.

xzyfer · 2016-11-03T23:54:25Z

Thanks @mcdado this seems sensible. I'll take a closer look this weekend.

mcdado · 2016-11-06T16:19:45Z

FWIW: this only bugs me because I try to do npm install --quiet --no-progress in my deployment scripts, but with node-sass in the dependencies, the silencing options don't work 😅
I get a lot of repeated lines from the download progress of the binary. I don't know if I changed all of the console statements or not.

xzyfer · 2016-11-06T22:51:42Z

That's interesting since the progress bar is already using npmlog
specifically so we can respect the no progress config. Have you got some
example output?

On 7 Nov 2016 3:19 AM, "David Gasperoni" [email protected] wrote:

FWIW: this only bugs me because I try to do npm install --quiet
--no-progress in my deployment scripts, but with node-sass in the
dependencies, the silencing options don't work 😅
I get a lot of repeated lines from the download progress of the binary. I
don't know if I changed all of the console statements or not.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1786 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAjZWGhqw9GpA3dyo_sEW9Tn970x1SXZks5q7f4igaJpZM4Ko-Mi
.

mcdado · 2016-11-06T23:39:19Z

Sure, here you go: link to video

mcdado · 2016-11-06T23:43:05Z

In deployment I have this command to build Sass in DeployBot:
npm install --quiet --no-progress --depth 0 1>/dev/null

Note that I even redirect stdout to /dev/null!

But in DeployBot's logs I still get all these lines:

[?25l[..................] - :
output [..................] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .................] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .................] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ................] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ...............] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ..............] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ..............] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .............] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ............] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ...........] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ...........] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ..........] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .........] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .........] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ........] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .......] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ......] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .....] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .....] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ....] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ...] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ..] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ..] - https://github.com/sass/node-sass/releases/download/v3.1
output [ .] - https://github.com/sass/node-sass/releases/download/v3.1
output [ ] - https://github.com/sass/node-sass/releases/download/v3.1
output �[?25h

xzyfer · 2016-11-06T23:51:07Z

Interesting, looks like a bug with how either we or npm handles --no-progress

nschonni · 2016-11-08T01:55:15Z

scripts/install.js

@@ -48,7 +48,7 @@ function download(url, dest, cb) {
    return response.statusCode >= 200 && response.statusCode < 300;
  };

-  console.log('Start downloading binary at', url);
+  log.http('Start downloading binary at', url);


I think this makes sense, but the messages need to be updated since the first parameter to the function in npmlog is the prefix rather than just contating the series of objects
EX:

log.http('install.js', 'Start downloading binary at ' + url)

mcdado · 2016-11-08T07:47:14Z

I hope it's okay that I used node-sass install. In case I can change it.

nschonni · 2016-11-09T20:35:25Z

scripts/install.js

@@ -48,7 +48,7 @@ function download(url, dest, cb) {
    return response.statusCode >= 200 && response.statusCode < 300;
  };

-  console.log('Start downloading binary at', url);
+  log.http('node-sass install', 'Start downloading binary at', url);


The , url doesn't work the same way as it did with console, so it either needs a temaplate in the message string or just switch it to + url

Sorry, that slipped when I replaced the parameters! I'll change it now.

Done in 182ff9d

Change console.log to npmlog

In for a penny in for a pound. I also noticed that #1786 resulted in some funky looking console output. I've taken this opportunity to tidy up our install script output whilst maintaining the information useful for debugging install issues. ``` > [email protected] install /Users/xzyfer/Projects/Sass/upstream/node-sass > node scripts/install.js http node-sass install Downloading binary from https://github.com/sass/node-sass/releases/download/v3.11.3/darwin-x64-47_binding.node http node-sass install Download complete info node-sass install Binary saved at /Users/xzyfer/Projects/Sass/upstream/node-sass/vendor/darwin-x64-47/binding.node > [email protected] postinstall /Users/xzyfer/Projects/Sass/upstream/node-sass > node scripts/build.js info node-sass build Binary found at /Users/xzyfer/Projects/Sass/upstream/node-sass/vendor/darwin-x64-47/binding.node info node-sass build Testing binary info node-sass build Binary is fine; exiting. ```

In for a penny in for a pound. I also noticed that #1786 resulted in some funky looking console output. I've taken this opportunity to tidy up our install script output whilst maintaining the information useful for debugging install issues. ``` > [email protected] install /Users/xzyfer/Projects/Sass/upstream/node-sass > node scripts/install.js http node-sass install Downloading binary from https://github.com/sass/node-sass/releases/download/v3.11.3/darwin-x64-47_binding.node http node-sass install Download complete info node-sass install Binary saved at /Users/xzyfer/Projects/Sass/upstream/node-sass/vendor/darwin-x64-47/binding.node > [email protected] postinstall /Users/xzyfer/Projects/Sass/upstream/node-sass > node scripts/build.js info node-sass build Binary found at /Users/xzyfer/Projects/Sass/upstream/node-sass/vendor/darwin-x64-47/binding.node info node-sass build Testing binary info node-sass build Binary is fine > [email protected] prepublish /Users/xzyfer/Projects/Sass/upstream/node-sass > not-in-install && node scripts/prepublish.js || in-install ```

We started using npmlog for install logging in #1786, then went all out in #1796. npmlog has caused some issues, namely #1805. It doesn't provided enough value for how troublesome it is. The problem sparked #1786 was resolved in 5c7fb7f.

mcdado force-pushed the console-log branch from 1dffda8 to 00e9a4e Compare November 6, 2016 16:10

nschonni suggested changes Nov 8, 2016

View reviewed changes

mcdado added 2 commits November 8, 2016 08:43

Change console.log to npmlog

670ba4f

Add prefix to npmlog statements

fa3992b

mcdado force-pushed the console-log branch from 00e9a4e to fa3992b Compare November 8, 2016 07:44

xzyfer approved these changes Nov 8, 2016

View reviewed changes

nschonni reviewed Nov 9, 2016

View reviewed changes

Fix log extra parameter

182ff9d

nschonni approved these changes Nov 9, 2016

View reviewed changes

xzyfer merged commit 40dd964 into sass:master Nov 11, 2016

mcdado deleted the console-log branch November 11, 2016 23:42

marvinhagemeister pushed a commit to marvinhagemeister/node-sass that referenced this pull request Nov 13, 2016

Switch from console.log to npmlog (sass#1786)

0d15fd8

Change console.log to npmlog

xzyfer mentioned this pull request Nov 13, 2016

Use npmlog for all install script output #1796

Merged

xzyfer mentioned this pull request Nov 15, 2016

Use console for logging instead of npmlog #1806

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch from console.log to npmlog #1786

Switch from console.log to npmlog #1786

mcdado commented Nov 3, 2016

xzyfer commented Nov 3, 2016

mcdado commented Nov 6, 2016

xzyfer commented Nov 6, 2016

mcdado commented Nov 6, 2016

mcdado commented Nov 6, 2016 •

edited

Loading

xzyfer commented Nov 6, 2016

nschonni Nov 8, 2016

mcdado commented Nov 8, 2016

nschonni Nov 9, 2016

mcdado Nov 9, 2016

mcdado Nov 9, 2016

Switch from console.log to npmlog #1786

Switch from console.log to npmlog #1786

Conversation

mcdado commented Nov 3, 2016

xzyfer commented Nov 3, 2016

mcdado commented Nov 6, 2016

xzyfer commented Nov 6, 2016

mcdado commented Nov 6, 2016

mcdado commented Nov 6, 2016 • edited Loading

xzyfer commented Nov 6, 2016

nschonni Nov 8, 2016

Choose a reason for hiding this comment

mcdado commented Nov 8, 2016

nschonni Nov 9, 2016

Choose a reason for hiding this comment

mcdado Nov 9, 2016

Choose a reason for hiding this comment

mcdado Nov 9, 2016

Choose a reason for hiding this comment

mcdado commented Nov 6, 2016 •

edited

Loading