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

feat(cli): Add --no-color support to argo lint. Fixes #12913 #12953

Merged
merged 1 commit into from
Apr 19, 2024
Merged

feat(cli): Add --no-color support to argo lint. Fixes #12913 #12953

merged 1 commit into from
Apr 19, 2024

Conversation

miltalex
Copy link
Contributor

@miltalex miltalex commented Apr 19, 2024 •
edited by agilgur5

Fixes #12913

Motivation

Visibility would be improved in cases where you capture the output, and include it in a GitHub comment on a Pull Request.

Modifications

Extend argo lint with the --no-color flag. When used the flag will disable the colors output by setting color.Toggle(false). Apart from that extended unit tests to cover the changes

Verification

  • Disable colors in Format func
  • Disable colors in Summarize func

@miltalex
Copy link
Contributor Author

@agilgur5 I believe that the failing test is due to #12906 . Any suggestions on how to proceed?

@shuangkun shuangkun added the area/cli The `argo` CLI label Apr 19, 2024
@miltalex miltalex marked this pull request as ready for review April 19, 2024 13:08
@agilgur5 agilgur5 self-assigned this Apr 19, 2024
@agilgur5 agilgur5 changed the title feat: Add --no-color support to argo lint. Fixes #12913 feat(cli): Add --no-color support to argo lint. Fixes #12913 Apr 19, 2024
@agilgur5 agilgur5 added this to the v3.6.0 milestone Apr 19, 2024
agilgur5
agilgur5 reviewed Apr 19, 2024
View reviewed changes
Copy link
Member

@agilgur5 agilgur5 left a comment •
edited

Choose a reason for hiding this comment

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

Thanks for adding this! Nice first PR 🙂

I have a tiny rename and small simplification in-line below

@agilgur5 I believe that the failing test is due to #12906 . Any suggestions on how to proceed?

Looks like it. I'll retry it myself once approved, so don't worry about it

cmd/argo/lint/formatter_pretty.go Outdated Show resolved Hide resolved
cmd/argo/lint/formatter_pretty.go Outdated Show resolved Hide resolved
cmd/argo/lint/formatter_pretty.go Outdated Show resolved Hide resolved
cmd/argo/commands/lint.go Show resolved Hide resolved
@miltalex
feat: Add --no-color support to argo lint. Fixes argoproj#12913
8c9375b 
Signed-off-by: Miltiadis Alexis <alexmiltiadis@gmail.com>
agilgur5
agilgur5 approved these changes Apr 19, 2024
View reviewed changes
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM

@agilgur5 agilgur5 merged commit 13d553c into argoproj:main Apr 19, 2024
27 checks passed
@miltalex miltalex deleted the feature/no-color-support branch April 19, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agilgur5 agilgur5 agilgur5 approved these changes

Assignees

@agilgur5 agilgur5

Labels
area/cli The `argo` CLI
Projects
None yet
Milestone
v3.6.0
Development

Successfully merging this pull request may close these issues.

Add --no-color support / ability to disable ANSI escape codes to argo lint command
3 participants
@miltalex @agilgur5 @shuangkun