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

wire through json/yaml print flags #61440

Merged
merged 1 commit into from Mar 25, 2018

Conversation

@juanvallejo
Member

juanvallejo commented Mar 20, 2018

Release note:

NONE

Begin implementing pieces needed to retrieve json, yaml printers from a set of flags.
Proposal: https://docs.google.com/document/d/19ZZFVe9oD1KQmk5uExggRWtRl_hKGfYnBXvHZJlgEro/edit#heading=h.pnvbfi14v4zz

cc @deads2k @soltysh @pwittrock

@soltysh

This comment has been minimized.

Contributor

soltysh commented Mar 21, 2018

/ok-to-test

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Mar 22, 2018

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@juanvallejo @soltysh

Pull Request Labels
  • sig/cli: Pull Request will be escalated to these SIGs if needed.
  • priority/important-longterm: Escalate to the pull request owners; move out of the milestone after 1 attempt.
  • kind/feature: New functionality.
Help
@soltysh

This comment has been minimized.

Contributor

soltysh commented Mar 22, 2018

/retest

@soltysh

One small nit and this is good to go

// Returns false if the specified outputFormat does not match a supported format.
// Supported Format types can be found in pkg/printers/printers.go
func (f *JSONYamlPrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, bool, error) {
if len(outputFormat) == 0 {

This comment has been minimized.

@soltysh

soltysh Mar 22, 2018

Contributor

Turn this into a nice switch like so:

outputFormat = strings.ToLower(outputFormat)
switch outputFormat {
case "json":
	return &JSONPrinter{}, true, nil
case "yaml":
	return &YAMLPrinter{}, true, nil
case "":
	return nil, false, fmt.Errorf("missing output format")
default
	return nil, false, nil
}

This comment has been minimized.

@juanvallejo

juanvallejo Mar 22, 2018

Member

Thanks, done

@soltysh

/lgtm
/approve no-issue

@k8s-ci-robot k8s-ci-robot added the lgtm label Mar 22, 2018

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Mar 22, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juanvallejo, soltysh

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

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 22, 2018

/test pull-kubernetes-integration

2 similar comments
@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 22, 2018

/test pull-kubernetes-integration

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 22, 2018

/test pull-kubernetes-integration

@fejta-bot

This comment has been minimized.

fejta-bot commented Mar 22, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

1 similar comment
@fejta-bot

This comment has been minimized.

fejta-bot commented Mar 23, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot

This comment has been minimized.

fejta-bot commented Mar 25, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Mar 25, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Mar 25, 2018

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-merge-robot k8s-merge-robot merged commit f8134de into kubernetes:master Mar 25, 2018

13 of 14 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-e2e-gce
Details
cla/linuxfoundation juanvallejo authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@juanvallejo juanvallejo deleted the juanvallejo:jvallejo/add-json-yaml-printer-flags branch Mar 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment