Skip to content

Conversation

@damemi
Copy link
Contributor

@damemi damemi commented Feb 2, 2018

Fix for #18016

Now oc status without -v will show a count for number of errors, warnings, and info identified. Also refactored this output to reduce redundant switch statements with including infos.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 2, 2018
@damemi
Copy link
Contributor Author

damemi commented Feb 5, 2018

/assign @enj

@damemi damemi changed the title Add infos count to oc status Issue 18016: Add infos count to oc status Feb 5, 2018
fmt.Fprintf(out, "%s identified, use '%[2]s status -v' to see details.\n", warnings, d.CommandBaseName)
switch {
case !d.Suggest && ((len(errorMarkers) > 0 && errorSuggestions > 0) || len(warningMarkers) > 0 || len(infoMarkers) > 0):
fmt.Fprintf(out, "%s identified, use '%s status -v' to see details.\n", markerString, d.CommandBaseName)
Copy link
Contributor

@juanvallejo juanvallejo Feb 5, 2018

Choose a reason for hiding this comment

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

I don't see where the count for each of these is printed.

EDIT: nevermind, see it now. Mind having a separate branch for a single info above?

if len(infoMarkers) == 1 {
    ...
} else if len(infoMarkers) > 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.

Sure, I wasn't sure if it grammatically made more sense to have, eg, "2 infos identified" vs "2 info identified"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Now `oc status` without `-v` will show a count for number of errors, warnings, and info identified. Also refactored this output to reduce redundant switch statements with including infos.
@damemi
Copy link
Contributor Author

damemi commented Feb 5, 2018

/retest

@juanvallejo
Copy link
Contributor

Changes look good to me
Tagging @soltysh for approval

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 5, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

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

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 5, 2018
@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue.

@openshift-merge-robot openshift-merge-robot merged commit ad4bd3f into openshift:master Feb 5, 2018
@damemi damemi deleted the iss18016-1 branch February 5, 2018 22:01
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. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants