Skip to content

Add format flag to report generate command#249

Merged
dimityrmirchev merged 3 commits intogardener:mainfrom
AleksandarSavchev:write-json-merged-reprot
Jun 11, 2024
Merged

Add format flag to report generate command#249
dimityrmirchev merged 3 commits intogardener:mainfrom
AleksandarSavchev:write-json-merged-reprot

Conversation

@AleksandarSavchev
Copy link
Copy Markdown
Member

@AleksandarSavchev AleksandarSavchev commented Jun 11, 2024

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

The `report generate` command now accepts a `--format` flag which determines the output format of the generated report. It can be set to one of `html` or `json`. Defaults to `html`.

Comment thread cmd/diki/app/app.go Outdated

func addReportGenerateFlags(cmd *cobra.Command, opts *generateOptions) {
cmd.PersistentFlags().Var(cliflag.NewMapStringString(&opts.distinctBy), "distinct-by", "If set generates a merged report. The keys are the IDs for the providers which the merged report will include and the values are distinct metadata attributes to be used as IDs for the different reports.")
cmd.PersistentFlags().StringVar(&opts.format, "format", "html", "Format for the merged report, dependent on 'distinct-by' flag to be set to take effect. Format can be 'html' or 'json'. Defaults to 'html'.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Cant we always have this flag taking an effect? If value is set to json without "--distinct-by" then we just write the report in json format. It does not make much sense to perform this operation, but it consistent. I do not like having the two flags that tightly connected.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For consistency we can add this option where when --format=json is set, but --distinct-by is not set we simply output the same json report as the input. I will now add it.

Copy link
Copy Markdown
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

/lgtm

@dimityrmirchev dimityrmirchev merged commit ea0466e into gardener:main Jun 11, 2024
@JordanJordanov JordanJordanov added the area/ipcei IPCEI (Important Project of Common European Interest) label Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ipcei IPCEI (Important Project of Common European Interest)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants