-
Notifications
You must be signed in to change notification settings - Fork 1.1k
named tuple toMap extension method #23827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
e908ff4
to
0387869
Compare
Will there be a |
0d8a36c
to
37b4759
Compare
I think |
37b4759
to
25bf111
Compare
@aherlihy could you please take a look? |
c89ac35
to
e5495a0
Compare
e5495a0
to
f884f31
Compare
f884f31
to
1253c3d
Compare
@road21 Is this ready for review? Once its rebased I will take a look. |
Also, at first glance, you should abstract over the implementation and return a Map (not ListMap), and name it |
1253c3d
to
f0b89fe
Compare
Yes, rebased
What about |
2532d5d
to
a94358c
Compare
Please, take a look. I have fixed method to |
910a534
to
6f0d4cc
Compare
6f0d4cc
to
254aa5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The failing CI test is unrelated. Please also squash the commits. It's a small change set.
d9da876
to
481ed88
Compare
481ed88
to
388b7e6
Compare
May I ask why? |
Just because we have an order and I don't see any downsides of |
There already are methods
toArray, toList
forNamedTuple
for collecting elements into collections.I would like to add method
toSeqMap
where keys are names of the elements.For example:
One obvious usage of this method is debugging code with named tuples (standard
toString
method doesn't show names):