Project

General

Profile

Backport #7936

backport r39449

Added by zzak (Zachary Scott) over 6 years ago. Updated about 3 years ago.

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

History

Updated by mame (Yusuke Endoh) over 6 years ago

  • Status changed from Open to Assigned
  • Assignee changed from mame (Yusuke Endoh) to nagachika (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

Updated by nagachika (Tomoyuki Chikanaga) over 6 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?

Updated by nobu (Nobuyoshi Nakada) over 6 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.

Updated by zzak (Zachary Scott) over 6 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?

Updated by nagachika (Tomoyuki Chikanaga) over 6 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from nagachika (Tomoyuki Chikanaga) to nobu (Nobuyoshi Nakada)

nobu, could you clarify about zzak's question?

Updated by nobu (Nobuyoshi Nakada) over 6 years ago

I meant for all methods.

Sorry for late reply.

Updated by zzak (Zachary Scott) over 6 years ago

nobu (Nobuyoshi Nakada) np, you mean something like:

attr_accessor(string or symbol)

or

attr(string or symbol)

#8

Updated by naruse (Yui NARUSE) about 3 years ago

  • Status changed from Feedback to Rejected

Also available in: Atom PDF