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.

Sign up for GitHub

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

Jump to bottom

fix: make sure noColor option is honored #3138

Merged
merged 21 commits into from
Sep 28, 2023
Merged

fix: make sure noColor option is honored #3138

merged 21 commits into from
Sep 28, 2023

Conversation

DanielMSchmidt
Copy link
Contributor

@DanielMSchmidt DanielMSchmidt commented Sep 20, 2023 •
edited

Related issue

CI run

Description

Makes sure no process can add color to output if noColor option is set

Checklist

  • I have updated the PR title to match CDKTF's style guide
  • I have run the linter on my code locally
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works if applicable
  • New and existing unit tests pass locally with my changes

@DanielMSchmidt DanielMSchmidt requested a review from a team as a code owner September 20, 2023 13:14
@DanielMSchmidt DanielMSchmidt requested review from ansgarm and Maed223 and removed request for a team September 20, 2023 13:14
@DanielMSchmidt DanielMSchmidt added priority/important-soon High priority, to be worked on as part of our current release or the following one. ci/run-integration and removed ci/run-unit/@cdktf/commons ci/run-unit/@cdktf/provider-generator ci/run-unit/@cdktf/cli-core ci/provider-integration priority/important-soon High priority, to be worked on as part of our current release or the following one. labels Sep 20, 2023
DanielMSchmidt and others added 18 commits September 28, 2023 15:38
@DanielMSchmidt @ansgarm
WIP
18166c6
@DanielMSchmidt @ansgarm
WIP
234c9cb
@DanielMSchmidt @ansgarm
chore: update uuid
876a07f
@DanielMSchmidt @ansgarm
WIP
d5ca837
@DanielMSchmidt @ansgarm
fix: pass noColor flag
184e473
@DanielMSchmidt @ansgarm
fix: use no-color in shell
dbf43ee
@DanielMSchmidt @ansgarm
fix: strip all ansis
77dd40c
@ansgarm
fix: stringify provider constraint to properly print object in log me…
710b8e0 
…ssage
@ansgarm
fix(cli): infer noColor from flag or env for calls like collectDebugI…
307ec26 
…nformation() that run before CLI commands
@ansgarm
fix(provider-generator): fix import after rebasing
9828a8c
@ansgarm
fix: run prettier
448806c
@ansgarm
fix: remove console log that breaks tests
a97acef
@ansgarm
fix: remove more stray console log statements
116f7e3
@ansgarm
wip - console log debugging
caf5421
@ansgarm
fix: fix test that relied on a mocked exec that didnt intend to mock …
1f41f51 
…all exec calls
@ansgarm
wip
b9a9076
@ansgarm
fix: logger should not print with colors if colors are disabled
0778643
@ansgarm
chore: remove old console logs
f18827e
@ansgarm ansgarm force-pushed the remove-colors-if-no-color-option-is-passed branch from 28368d7 to f18827e Compare September 28, 2023 13:38
Maed223
Maed223 approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@Maed223 Maed223 left a comment

Choose a reason for hiding this comment

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

Looks good 💯

Have a curiosity about the console.logs added to the tests, but its non-blocking.

test/csharp/no-color-command-option-test/test.ts Show resolved Hide resolved
@ansgarm ansgarm added this pull request to the merge queue Sep 28, 2023
Merged via the queue into main with commit 3d3ae7a Sep 28, 2023
161 checks passed
@ansgarm ansgarm deleted the remove-colors-if-no-color-option-is-passed branch September 28, 2023 14:19
@ansgarm ansgarm mentioned this pull request Oct 4, 2023
Merged
@github-actions
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ansgarm ansgarm ansgarm approved these changes

@Maed223 Maed223 Maed223 approved these changes

Assignees

@ansgarm ansgarm

Labels
ci/provider-integration ci/run-integration ci/run-unit/@cdktf/cli-core ci/run-unit/@cdktf/commons ci/run-unit/@cdktf/provider-generator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@DanielMSchmidt @ansgarm @Maed223 @team-tf-cdk