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

Switch human logging to stderr to enable pipeable validation output #416

Merged
merged 7 commits into from
Apr 18, 2023

Conversation

colindean
Copy link
Contributor

Work toward go-vela/community#313

@colindean colindean requested a review from a team as a code owner February 28, 2023 23:10
@colindean
Copy link
Contributor Author

N.b. I did this with github.dev so no idea if the tests work… it was a copy-paste job for the stdout -> stderr impl

@codecov
Copy link

codecov bot commented Feb 28, 2023

Codecov Report

Merging #416 (0555f52) into main (7000344) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #416      +/-   ##
==========================================
+ Coverage   86.00%   86.04%   +0.03%     
==========================================
  Files         132      133       +1     
  Lines        5754     5768      +14     
==========================================
+ Hits         4949     4963      +14     
  Misses        660      660              
  Partials      145      145              
Impacted Files Coverage Δ
action/pipeline/validate.go 88.72% <100.00%> (ø)
internal/output/stderr.go 100.00% <100.00%> (ø)

internal/output/stderr.go Outdated Show resolved Hide resolved
internal/output/stderr_test.go Outdated Show resolved Hide resolved
@colindean
Copy link
Contributor Author

I'm trying to write a pre-commit hook and stumbled upon this muddying the YAML output from vela validate pipeline.

@ecrupper
Copy link
Contributor

@colindean So is the point of this PR to have the entire stdout be the YAML output of the pipeline and send the is valid statement to stderr? If so, I think I'd almost prefer removing that is valid statement in favor of just the compiled pipeline instead of sending them to different places.

@colindean
Copy link
Contributor Author

If I had it my way:

  • human-readable output always on stderr
  • machine-readable output always on stdout
  • machine-readable output emitted only when opted-in, e.g. --output-validated [filename or -, defaults to stdout]

This PR accomplishes the first two.

Copy link
Contributor

@ecrupper ecrupper left a comment

Choose a reason for hiding this comment

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

A couple typos. Tested this locally:

vela validate pipeline | yq -j

Error: bad file '-': yaml: line 2: mapping values are not allowed in this context

vs.

~/.../cli/release/darwin/amd64/vela validate pipeline | yq -j 

{
  "version": "1",
  "metadata": {
    "clone": false,
    "environment": [
      "steps",
      "services",
      "secrets"
    ]
  },
.... etc

Nice

action/pipeline/validate.go Outdated Show resolved Hide resolved
internal/output/stderr.go Outdated Show resolved Hide resolved
ecrupper and others added 2 commits April 18, 2023 12:07
Co-authored-by: Easton Crupper <65553218+ecrupper@users.noreply.github.com>
@ecrupper ecrupper merged commit d77d1db into go-vela:main Apr 18, 2023
@colindean colindean deleted the stderr branch April 19, 2023 16:14
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.

None yet

3 participants