-
-
Notifications
You must be signed in to change notification settings - Fork 627
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
fix: respect --color
/--no-color
arguments
#2042
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2042 +/- ##
==========================================
+ Coverage 68.50% 68.65% +0.14%
==========================================
Files 76 76
Lines 2413 2418 +5
Branches 497 497
==========================================
+ Hits 1653 1660 +7
+ Misses 760 758 -2
Continue to review full report at Codecov.
|
1615bfa
to
2faaaa1
Compare
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.
/cc @webpack/cli-team
Good job, thanks
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.
oh, what about logger? Because --no-colors
should disable colors for logger too
I will update 👍🏻 |
60cd78d
to
7d03569
Compare
I will fix the CI problems soon. |
232dea6
to
d30018f
Compare
/cc @webpack/cli-team |
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.
/cc @webpack/cli-team
Good to merge? |
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.
/cc @webpack/cli-team
Thanks for waiting, I'm on it 👍 |
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.
minor nits
Co-authored-by: Anshuman Verma <[email protected]>
@snitin315 Thanks for your update. I labeled the Pull Request so reviewers will review it again. @anshumanv Please review the new changes. |
/cc @webpack/cli-team focus on it, it is blocker for other refactor arg parser |
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.
Thanks
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.
Feel free to merge after CI green
random fail on macos, let's ignore, we will return to this if it happens again |
--color
/--no-color
arguments
What kind of change does this PR introduce?
bugfix
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
no
Summary
stats
colors should be disabled if we use--no-color
.Fixes #2038 as well
Before
After
Does this PR introduce a breaking change?
Nope
Other information
No