Project

General

Profile

Backport #7936

backport r39449

Added by Zachary Scott almost 4 years ago. Updated 6 months ago.

Status:
Rejected
Priority:
Normal
[ruby-core:52780]

History

#1 [ruby-core:52783] Updated by Yusuke Endoh almost 4 years ago

  • Status changed from Open to Assigned
  • Assignee changed from Yusuke Endoh to Tomoyuki Chikanaga

This is not a typo fix or format improvement, but includes (very small) semantic change.
Let's consider after the patch level release after p0.

--
Yusuke Endoh mame@tsg.ne.jp

#2 [ruby-core:53267] Updated by Tomoyuki Chikanaga over 3 years ago

I think passing String to these methods is meaningful because it is a way to avoid unnecessary Symbol allocation. So I'll merge r39449.

mame san, do you have a concern?

#3 [ruby-core:53712] Updated by Nobuyoshi Nakada over 3 years ago

Yes, it's meaningful, but the documents are inaccurate on that point.
Any *_defined? methods don't convert the argument to a Symbol always.

I don't think the distinguishment between symbol and string here is important enough.
And the name symbol/string seems less meaningful.
I'd suggest to change the argument names and mention that it can be a symbol or a string.

#4 [ruby-core:53720] Updated by Zachary Scott over 3 years ago

nobu (Nobuyoshi Nakada) wrote:

Yes, it's meaningful, but the documents are inaccurate on that point.

Thank you for your feedback, always appreciated nobu-san.

I'd suggest to change the argument names and mention that it can be a symbol or a string.

Do you mean for *_defined? methods or for all methods mentioned in this patch?

#5 [ruby-core:54257] Updated by Tomoyuki Chikanaga over 3 years ago

  • Assignee changed from Tomoyuki Chikanaga to Nobuyoshi Nakada
  • Status changed from Assigned to Feedback

nobu, could you clarify about zzak's question?

#6 [ruby-core:54611] Updated by Nobuyoshi Nakada over 3 years ago

I meant for all methods.

Sorry for late reply.

#7 [ruby-core:54613] Updated by Zachary Scott over 3 years ago

@nobu np, you mean something like:

attr_accessor(string or symbol)

or

attr(string or symbol)

#8 Updated by Yui NARUSE 6 months ago

  • Status changed from Feedback to Rejected

Also available in: Atom PDF