Backport #7936

backport r39449

Added by Zachary Scott about 1 year ago. Updated 12 months ago.

[ruby-core:52780]
Status:Feedback
Priority:Normal
Assignee:Nobuyoshi Nakada

History

#1 Updated by Yusuke Endoh about 1 year 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 Updated by Tomoyuki Chikanaga about 1 year 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 Updated by Nobuyoshi Nakada about 1 year 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 Updated by Zachary Scott about 1 year 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 Updated by Tomoyuki Chikanaga about 1 year ago

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

nobu, could you clarify about zzak's question?

#6 Updated by Nobuyoshi Nakada 12 months ago

I meant for all methods.

Sorry for late reply.

#7 Updated by Zachary Scott 12 months ago

@nobu np, you mean something like:

attr_accessor(string or symbol)

or

attr(string or symbol)

Also available in: Atom PDF