Support keyword arguments on all Concurrent Struct classes #738
Conversation
|
Awesome! Let me know when I can have a look. |
pitr-ch
changed the title
pitr-ch
changed the title
ghost
added
the
|
@pitr-ch 👌 – this would definitely make MRI > 2 and other analogues a requirement – otherwise Also if all looks good I'll squash the commit history 👍 |
|
I've just merged #740, could you rebase on master? Squashing is not necessary. |
numeraltwo
force-pushed
the
keyword_parameters_for_structs
branch
from
67c3c5a
to
06f8a02
Compare
|
@pitr-ch all set. |
| @@ -71,20 +71,20 @@ def select(&block) | |||
| end | |||
|
|
|||
| # @!macro struct_new | |||
| def self.new(*args, &block) | |||
| def self.new(*args, **kw_args, &block) | |||
There was a problem hiding this comment.
It would be better to have the signature more specific def self.new(*args, keyword_init: false, &block). Afaik no other keyword argument has any effect.
| @@ -197,20 +197,20 @@ def []=(member, value) | |||
| end | |||
|
|
|||
| # @!macro struct_new | |||
There was a problem hiding this comment.
Please update the struct_new definition to include documentation about keyword_init option.
| ns_initialize(*values, **kw_values) | ||
| end | ||
|
|
||
| # @!macro [attach] struct_keyword_init |
There was a problem hiding this comment.
The macro definition at the beginning of the can doc can be omitted, if the doc is not shared with other methods.
| # | ||
| # @return [Boolean] true if struct uses keyword arguments | ||
| def keyword_init? | ||
| self.class::KEYWORD_INIT | ||
| end |
There was a problem hiding this comment.
The keyword_init? on struct instance probably has not usage other then the initialization, I think it can be omitted or moved to the struct class replacing the KEYWORD_INIT constant.
|
@numeraltwo friendly reminder :) |
|
Closing both due to inactivity, and also because there's a proper way to delegate keyword arguments now. Please do re-engage if you still want to work on it. |
📓 Original issue: #694
(fix #694)