Skip to content

ext: add list.sort()#1021

Merged
TristonianJones merged 6 commits into
cel-expr:masterfrom
cezar-guimaraes:list-sort
Sep 23, 2024
Merged

ext: add list.sort()#1021
TristonianJones merged 6 commits into
cel-expr:masterfrom
cezar-guimaraes:list-sort

Conversation

@cezar-guimaraes

Copy link
Copy Markdown
Contributor

I skipped creating an issue because this seemed like a small enough change to me, but please let me know if an issue is preferable.

This PR adds a <list(T)>.sort() -> <list(T)> member. T must implement (traits.Comparer).

I believe that the check if the element type implements traits.Comparer only happens at runtime (how can I write tests for this?). An alternative implementation would create separate member overloads for list(T), T in {set of comparable types} similar to kube apiserver list extensions

Let me know if I should update the PR to create separate overloads.

@google-cla

google-cla Bot commented Sep 12, 2024

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@TristonianJones

Copy link
Copy Markdown
Collaborator

/gcbrun

Comment thread ext/lists.go Outdated
@TristonianJones

Copy link
Copy Markdown
Collaborator

/gcbrun

Comment thread ext/lists.go Outdated
@TristonianJones

Copy link
Copy Markdown
Collaborator

@cezar-guimaraes I just had one more comment, but otherwise this is looking good.

@TristonianJones

Copy link
Copy Markdown
Collaborator

/gcbrun

Comment thread ext/README.md
@cezar-guimaraes cezar-guimaraes force-pushed the list-sort branch 2 times, most recently from eb55f76 to 50e5a97 Compare September 23, 2024 20:43
@cezar-guimaraes

Copy link
Copy Markdown
Contributor Author

Rebased the branch to use the new helper type introduced in #1026
1ad695c

@TristonianJones

Copy link
Copy Markdown
Collaborator

/gcbrun

@TristonianJones TristonianJones merged commit 45bd6f6 into cel-expr:master Sep 23, 2024
@TristonianJones

Copy link
Copy Markdown
Collaborator

@cezar-guimaraes thanks for your contribution and your patience!

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.

2 participants