Skip to content

Improve provider meta sorting#37

Merged
dimityrmirchev merged 2 commits intogardener:mainfrom
dimityrmirchev:sort-meta
Oct 4, 2023
Merged

Improve provider meta sorting#37
dimityrmirchev merged 2 commits intogardener:mainfrom
dimityrmirchev:sort-meta

Conversation

@dimityrmirchev
Copy link
Copy Markdown
Member

What this PR does / why we need it:
Sorts the metadata keys and providers in order to generate more consistent reports.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Metadata and providers are now sorted when generating a report in order to improve consistency and readability.

@dimityrmirchev dimityrmirchev requested a review from a team as a code owner October 3, 2023 13:29
@gardener-robot gardener-robot added needs/review size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 3, 2023
@ghost ghost added the reviewed/ok-to-test label Oct 3, 2023
Copy link
Copy Markdown
Member

@AleksandarSavchev AleksandarSavchev left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! One minor nit.

Comment on lines 62 to 65
{{ $meta := MergedMetadataTexts . }}
{{ $keys := SortedMapKeys $meta }}
{{ range $id := $keys }}<li><span class="font-bold">{{ $id }}</span> {{ index $meta $id }}</li>
{{ end }}
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.

Suggested change
{{ $meta := MergedMetadataTexts . }}
{{ $keys := SortedMapKeys $meta }}
{{ range $id := $keys }}<li><span class="font-bold">{{ $id }}</span> {{ index $meta $id }}</li>
{{ end }}
{{- $meta := MergedMetadataTexts . }}
{{- $keys := SortedMapKeys $meta }}
{{- range $id := $keys }}
<li><span class="font-bold">{{ $id }}</span> {{ index $meta $id }}</li>
{{- end }}

Use dash to remove unnecessary new lines from report.

Comment thread pkg/report/templates/html/report.html Outdated
Comment on lines 62 to 65
{{ $keys := SortedMapKeys .Metadata }}
{{ $meta := .Metadata }}
{{ range $key := $keys }}<li><span class="font-semibold">{{ $key }}</span>: {{ index $meta $key }}</li>
{{ end }}
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.

Suggested change
{{ $keys := SortedMapKeys .Metadata }}
{{ $meta := .Metadata }}
{{ range $key := $keys }}<li><span class="font-semibold">{{ $key }}</span>: {{ index $meta $key }}</li>
{{ end }}
{{- $keys := SortedMapKeys .Metadata }}
{{- $meta := .Metadata }}
{{- range $key := $keys }}
<li><span class="font-semibold">{{ $key }}</span>: {{ index $meta $key }}</li>
{{- end }}

Same as above.

@gardener-robot gardener-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 4, 2023
Copy link
Copy Markdown
Member

@AleksandarSavchev AleksandarSavchev left a comment

Choose a reason for hiding this comment

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

/lgtm

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

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants