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

feat: add verbose flag for pipeline values #861

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

wyardley
Copy link
Contributor

@wyardley wyardley commented Mar 8, 2023

Checklist

=========

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked for similar issues and haven't found anything relevant.
  • This is not a security issue (which should be reported here: https://circleci.com/security/)
  • I have read Contribution Guidelines.

Internal Checklist

  • I am requesting a review from my own team as well as the owning team
  • I have a plan in place for the monitoring of the changes that I am making (this can include new monitors, logs to be aware of, etc...)

Changes

=======

  • Add a -v / --verbose flag to enable verbose output
  • Print the config values one line at a time, and add newlines
  • Write to stderr so that outputting to a file won't generate invalid YAML when writing files to stdout

Rationale

The current output is not aesthetically pleasing, and is also hard to read / follow. Additionally, people may want a way to suppress this new behavior

@particleflux also pointed out in #860 that in the circleci config process and circleci config pack commands, this will actually generate invalid YAML.

Fixes #860

Considerations

Happy to adjust the option name / invert the behavior to make things more consistent with options elsewhere in the program, or to tweak the formatting / output slightly. I think this is, while maybe not perfect, a big improvement.

Screenshots

Before

image

After

image

@wyardley wyardley requested a review from a team as a code owner March 8, 2023 00:20
cmd/config.go Outdated Show resolved Hide resolved
@wyardley wyardley force-pushed the wyardley/860 branch 2 times, most recently from 08875a4 to 3aa6bb4 Compare March 13, 2023 16:13
@wyardley wyardley changed the title feat: allow hiding pipeline values in validation output feat: add verbose flag for pipeline values Mar 17, 2023
@wyardley
Copy link
Contributor Author

@elliotforbes now that it's updated, are you able to review this and / or bug someone who can?

Copy link

@goetzc goetzc left a comment

Choose a reason for hiding this comment

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

LGTM!

@elliotforbes
Copy link
Contributor

@elliotforbes now that it's updated, are you able to review this and / or bug someone who can?

Let me ping the team, I've got another PR for this repo I'm looking to get out first which will likely cause conflicts.

- Add a `-v` / `--verbose` flag to enable verbose output
- Print the config values one line at a time, and add newlines
- Write to stderr so that outputting to a file won't generate invalid
  YAML when writing files to stdout
Fixes CircleCI-Public#860
@wyardley
Copy link
Contributor Author

@elliotforbes rebased

Copy link
Contributor

@abdelDriowya abdelDriowya left a comment

Choose a reason for hiding this comment

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

LGTM

@elliotforbes
Copy link
Contributor

@wyardley - feel free to merge when ready, I'll cut a new release branch and get this out today hopefully!

@wyardley
Copy link
Contributor Author

@elliotforbes i think someone with commit access will have to merge.

@elliotforbes elliotforbes merged commit 651f295 into CircleCI-Public:develop Mar 23, 2023
@wyardley wyardley mentioned this pull request Mar 30, 2023
15 tasks
wyardley added a commit to wyardley/circleci-cli that referenced this pull request Mar 30, 2023
Reintroduces the output style introduced in CircleCI-Public#861 to resolve CircleCI-Public#860, but
reverted in CircleCI-Public#884
wyardley added a commit to wyardley/circleci-cli that referenced this pull request Apr 5, 2023
Resolve some issues from CircleCI-Public#861 and CircleCI-Public#896

- Suppress some whitespace when verbose output is not enabled
- Write to stderr again
- Use "%v" instead of "%s" so that "number" in pipeline values gets
  printed correctly
- Sort values to provide stable output order for pipeline values
wyardley added a commit to wyardley/circleci-cli that referenced this pull request Apr 5, 2023
Resolve some issues from CircleCI-Public#861 and CircleCI-Public#896

- Suppress some whitespace when verbose output is not enabled
- Write to stderr again
- Use "%v" instead of "%s" so that "number" in pipeline values gets
  printed correctly
- Sort values to provide stable output order for pipeline values
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.

messy validation output
4 participants