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

Disable colors for non-interactive output #481

Merged

Conversation

c0d3d
Copy link
Contributor

@c0d3d c0d3d commented Jan 30, 2019

What did you implement:

Closes #480

How did you implement it:

Check if stdout is a tty when deciding to use colors or not.

How can we verify it:

  1. Run from tty (interactive shell) to see default still has colors
  2. Pipe into a file to see that it doesn't write colors.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@c0d3d c0d3d force-pushed the no-colors-for-non-interactive branch from 40eb8d6 to 720b17b Compare January 30, 2019 15:38
@c0d3d
Copy link
Contributor Author

c0d3d commented Mar 17, 2019

@HyperBrain Can you look at this? Would help our Jenkins build from looking awful.
Thanks!

Copy link
Contributor

@hassankhan hassankhan left a comment

Choose a reason for hiding this comment

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

Perhaps I'm missing something, but it doesn't seem like the tty package is in the project's package.json?

Disregard 😄

@c0d3d
Copy link
Contributor Author

c0d3d commented Mar 29, 2019

@hassankhan Can you help get this merged?
Would make my Jenkins output much better :D

@hassankhan
Copy link
Contributor

hassankhan commented Mar 31, 2019

@c0d3d Unfortunately I don't have write access to the repo, I would love to see this PR merged for the same reason 👍

@HyperBrain is there anything I can do to help get the PR merged in? 😄

@HyperBrain
Copy link
Member

@hassankhan I just invited you to the serverless-webpack-team. You should have write access as soon as you confirm the request ;-)

lib/compile.js Show resolved Hide resolved
@HyperBrain HyperBrain modified the milestones: 5.2.1, 5.3.0 Apr 7, 2019
@hassankhan
Copy link
Contributor

hassankhan commented Apr 7, 2019

Thanks for the invite @HyperBrain 😄

If there's no other issue with the PR, is it cool to go ahead and merge it? Or is there any other process I should be following aside from code-review + passing tests?

@HyperBrain
Copy link
Member

HyperBrain commented Apr 7, 2019 via email

@hassankhan hassankhan merged commit 57957ce into serverless-heaven:master Apr 7, 2019
@hassankhan
Copy link
Contributor

Thanks for creating the PR @c0d3d, should be released in v5.3.0 👍

@c0d3d c0d3d deleted the no-colors-for-non-interactive branch April 7, 2019 22:25
jamesmbourne pushed a commit to jamesmbourne/serverless-webpack that referenced this pull request Oct 15, 2019
…-interactive

Disable colors for non-interactive output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default color output should depend on TTY
3 participants