Skip to content

Conversation

a-hilaly
Copy link
Member

@a-hilaly a-hilaly commented Jul 6, 2021

Issue N/A

This patch adds a new option Index for print column configuration.
Setting an index for a field and resource.print.sortBy to index will
intruct the code generator to sort the kubebuilder:printcolumns by the
Index value.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This patch adds a new option `Index` for print column configuration.
Setting an index for a field and `resource.print.sortBy` to `index` will
intruct the code generator to sort the `kubebuilder:printcolumns` by the
`Index` value.
@ack-bot ack-bot requested review from jaypipes and vijtrip2 July 6, 2021 16:25
Comment on lines +80 to +83
case "index":
return func(a, b *PrinterColumn) bool {
return a.Index < b.Index
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be the first case in the switch? Sorting by index should be preferred over any other?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't how moving this case could change the default/preferred value. sortByField will have one of the values.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you mean moving it to the default case? or maybe somehow set a default for $resource.print.sortBy ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh no I totally misread this method. Sorry

@RedbackThomson
Copy link
Contributor

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 7, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Jul 7, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, RedbackThomson

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:
  • OWNERS [A-Hilaly,RedbackThomson]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit e050b9f into aws-controllers-k8s:main Jul 7, 2021
RedbackThomson pushed a commit to RedbackThomson/ack-code-generator that referenced this pull request Jul 8, 2021
…ws-controllers-k8s#124)

Issue N/A

This patch adds a new option `Index` for print column configuration.
Setting an index for a field and `resource.print.sortBy` to `index` will
intruct the code generator to sort the `kubebuilder:printcolumns` by the
`Index` value.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants