Skip to content

Conversation

chunglu-chou
Copy link
Contributor

To better support the listing status functionality, the following things are updated:

  1. Add inventory name as an attribute in both event and json output format so users are able to see which inventory the resource comes from.
  2. Support status filtering function on event and json format.
  3. Remove unused functions and constants.

}
switch ep.Format {
case printers.EventsPrinter:
_, err := fmt.Printf("%s/%s/%s/%s is %s: %s\n", invName,
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt.Printf prints to os.Stdout directly. However, the underlying cli-utils library prints to ioStreams.Out, which is os.Stdout in most cases, but it can also be other streams.

I guess we need to wait for @mortent on how to proceed with the deviated fork. If we decide to go with the major changes in the forked cli-utils, it's better to pass along ioStreams for printing.

@@ -77,7 +73,7 @@ func NewRunner(ctx context.Context, factory util.Factory) *Runner {
"How long to wait before exiting")
c.Flags().BoolVar(&r.list, "list", false, "List status of all packages or not")
c.Flags().StringVar(&r.inventoryNames, "inv-name", "", "names of targeted inventory: inv1,inv2,...")
c.Flags().StringVar(&r.namespaces, "namespace", "", "names of targeted namespaces: ns1,ns2,...")
c.Flags().StringVar(&r.namespaces, "namespaces", "", "names of targeted namespaces: ns1,ns2,...")
c.Flags().StringVar(&r.status, "status", "", "targeted status: st1,st2...")
Copy link
Contributor

Choose a reason for hiding this comment

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

Use plural for consistency.

…mespace, and inventory name. Add a new column in table format showing the inventory name.
@droot
Copy link
Contributor

droot commented Jul 25, 2022

@karlkfi @mortent will be the best person to review this.

@karlkfi
Copy link
Contributor

karlkfi commented Jul 25, 2022

Is this a dupe of #3345?

@chunglu-chou
Copy link
Contributor Author

@karlkfi No, this one supports all output formats, while #3345 only support table format.

@karlkfi
Copy link
Contributor

karlkfi commented Jul 25, 2022

Is this intended to be merged after #3345 or should we close #3345 as obsolete because this one is more complete?

@chunglu-chou
Copy link
Contributor Author

@karlkfi Yes it is intended to be merged after #3345, the original purpose of these separate PRs is to have less changes in a single PR.

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.

5 participants