Skip to content
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

reduce effect of npm silent loglevel #2706

Closed
wants to merge 12 commits into from
Closed

reduce effect of npm silent loglevel #2706

wants to merge 12 commits into from

Conversation

kuceb
Copy link
Contributor

@kuceb kuceb commented Nov 1, 2018

fix #2705

changes:

  • npm i cypress --silent has Listr verbose renderer: (previously there were no console logs)
    but, we could change this to use default renderer
Installing Cypress (version: 1.2.3)
[xx:xx:xx]  Downloading Cypress     [started]
[xx:xx:xx]  Downloading Cypress     [completed]
[xx:xx:xx]  Unzipping Cypress       [started]
[xx:xx:xx]  Unzipping Cypress       [completed]
[xx:xx:xx]  Finishing Installation  [started]
[xx:xx:xx]  Finishing Installation  [completed]
  • cypress verify ignores --silent
[xx:xx:xx]  Verifying Cypress can run /cache/Cypress/1.2.3/Cypress.app [started]
[xx:xx:xx]  Verifying Cypress can run /cache/Cypress/1.2.3/Cypress.app [completed]
 Opening Cypress...
  • calls to logger.log() are no longer silenced, only calls to logger.info, so cypress version, cypress cache list, etc will not be silenced by npm_config_loglevel

@jennifer-shehane
Copy link
Member

@bkucera Merge conflict

@brian-mann
Copy link
Member

I understand the changes made to the cache stuff... but why did you additionally removing silencing of cypress install?

I'm pretty sure that was the point of using --silence - to prevent the gazillions of installing Cypress lines that are added in situations where we don't use the listr renderer and instead write messages like it's in a TTY.

@brian-mann brian-mann self-requested a review November 1, 2018 19:22
Copy link
Member

@brian-mann brian-mann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment

@kuceb
Copy link
Contributor Author

kuceb commented Nov 1, 2018

I understand the changes made to the cache stuff... but why did you additionally removing silencing of cypress install?

I'm pretty sure that was the point of using --silence - to prevent the gazillions of installing Cypress lines that are added in situations where we don't use the listr renderer and instead write messages like it's in a TTY.

@brian-mann
That's why I use the verbose listr renderer, which will max out at 6 lines of stdout

However, I think we could get away with just the

Installing Cypress (version: 1.2.3)

here's puppeteer:
pup

@jennifer-shehane
Copy link
Member

Hey @bkucera, could you address the merge conflict here and follow up with any leftover work to merge this in or close if stale. Thanks!

@kuceb
Copy link
Contributor Author

kuceb commented Jan 16, 2019

fixed merge conflict, I think this should be merged.

The comment that @brian-mann had was that with these changes we go from 0 lines of stdout with --silent to 6.

I think that's a fine amount of stdout for --silent, personally. It's not "gazillion"

@jennifer-shehane jennifer-shehane self-requested a review July 13, 2019 02:49
@cypress
Copy link

cypress bot commented Oct 1, 2019



Test summary

3584 0 47 0


Run details

Project cypress
Status Passed
Commit 964a66f
Started Dec 18, 2019 2:28 PM
Ended Dec 18, 2019 2:33 PM
Duration 04:55 💡
OS Linux Debian - 9.11
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@jennifer-shehane jennifer-shehane requested a review from a team January 8, 2020 08:21
@jennifer-shehane jennifer-shehane requested review from a team and flotwig and removed request for a team February 10, 2020 16:55
@jennifer-shehane
Copy link
Member

Closing due to inactivity. Please reopen or open a new pull request to address the changes necessary in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

npm_config_loglevel env variable is silencing too much (--silent)
3 participants