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

support json and yaml format outputs #31

Merged
merged 1 commit into from Mar 11, 2021

Conversation

ykakarap
Copy link
Contributor

@ykakarap ykakarap commented Mar 2, 2021

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

This PR add support for formatted outputs.
The following formats are supported:

  • JSON
  • YAML
  • LOG (Default)

Zeitgiest can be used by other systems, like build systems, to form further actions on the outputs.
Providing machine readable outputs allows zeitgeitst's outputs to be consumed by downstream settings.

Adds --output-format to specify the format of the output. Supported values are yaml, json and log (default).
Adds --output-file to specify the file if format is yaml or json. Defaults to dependencies_output.(yaml|json).

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Yes

Support for machine readable outputs. Supports JSON and YAML

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Mar 2, 2021
@k8s-ci-robot k8s-ci-robot added sig/release Categorizes an issue or PR as relevant to SIG Release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 2, 2021
@ykakarap ykakarap changed the title support json and yaml format outputs [WIP] support json and yaml format outputs Mar 2, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 2, 2021
@ykakarap ykakarap changed the title [WIP] support json and yaml format outputs support json and yaml format outputs Mar 2, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 2, 2021
@ykakarap
Copy link
Contributor Author

ykakarap commented Mar 2, 2021

/cc @cpanato
/cc @justaugustus

@@ -21,6 +21,12 @@ import (
"github.com/pkg/errors"
)

type VersionUpdate struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a note that this is also the external schema for the JSON/YAML output.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

version: 0.0.1
upstream:
flavour: dummy
url: example/example
Copy link
Member

Choose a reason for hiding this comment

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

nit: new line

cmd/validate.go Outdated
}

if o.outputFile != "" && o.outputFormat == "" {
logrus.Warnf("ignoring --output-file as not --output-format is not specified")
Copy link
Member

Choose a reason for hiding this comment

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

but we can output to a file the log format or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.
As mentioned in the description of the --output-file flag, it is only used if the output format is yaml or json.
If the output-format is log it will print the output to stdout (as before).

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

a few nits and comments
thanks for doing this!

dependencies/versions.go Outdated Show resolved Hide resolved
@justaugustus
Copy link
Member

justaugustus commented Mar 9, 2021

/hold for #36 and #38

EDIT: Both PRs have merged. We can cancel the hold once this has been rebased and adds the export command, as discussed w/ @ykakarap on Slack.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2021
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2021
@justaugustus justaugustus added this to In progress in Release Engineering (k-sigs) via automation Mar 9, 2021
@ykakarap ykakarap force-pushed the formatted_output branch 2 times, most recently from 5180d62 to cd910d8 Compare March 11, 2021 21:21
@ykakarap
Copy link
Contributor Author

/test pull-zeitgeist-verify

This command support formatted output. Supports JSON and YAML.
@ykakarap
Copy link
Contributor Author

@justaugustus rebased and now implements the logic under a new export command.

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 11, 2021
@justaugustus
Copy link
Member

@ykakarap -- Thanks so much for this functionality!
I have some microscopic nits here, but as mentioned on Slack, I'm going to work them into the next refactor.

/hold cancel
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 11, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justaugustus, ykakarap

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2021
@k8s-ci-robot k8s-ci-robot merged commit 53633f7 into kubernetes-sigs:master Mar 11, 2021
Release Engineering (k-sigs) automation moved this from In progress to Done Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Development

Successfully merging this pull request may close these issues.

None yet

5 participants