JRuby (please use github issues at http://bugs.jruby.org)
  1. JRuby (please use github issues at http://bugs.jruby.org)
  2. JRUBY-6227

[1.9] Struct#members and Struct::members should return an Array of Symbols in 1.9

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: JRuby 1.6.6, JRuby 1.7.0.pre1
    • Component/s: None
    • Labels:
      None
    • Patch Submitted:
      Yes
    • Number of attachments :
      1

      Description

      Hello,

      This is my first patch to JRuby, so please be teach me where I'm wrong.

      I noticed Struct.members return an Array of Strings, even in 1.9.
      A patch is attached, it removes the RubySpec tag which was "hiding" this issue.

      I'm unsure if the duplication between members18 and members19 is fine, but as it is mostly a loop, it's probably fine.
      I used the members18 method in unmarshalFrom(), maybe using the 19 version would be faster since it does not require to convert members to ruby Strings?

      Hope it helps,
      Benoit Daloze

        Activity

        Hide
        Hiro Asari added a comment -

        Benoit,

        Thanks for the patch. In general, we don't put the suffix '18' for methods in the 1.8 mode. This is making your patch noisier than necessary, I believe.

        Show
        Hiro Asari added a comment - Benoit, Thanks for the patch. In general, we don't put the suffix '18' for methods in the 1.8 mode. This is making your patch noisier than necessary, I believe.
        Hide
        Benoit Daloze added a comment -

        Here is the updated patch, which do not rename members to members18.
        Indeed, the patch looks cleaner, thanks for your advice.

        I also used members19() in unmarshalFrom() since it avoids converting the members to RubyStrings.

        Show
        Benoit Daloze added a comment - Here is the updated patch, which do not rename members to members18. Indeed, the patch looks cleaner, thanks for your advice. I also used members19() in unmarshalFrom() since it avoids converting the members to RubyStrings.
        Hide
        Hiro Asari added a comment -

        Benoit,

        Thank you for updating the patch. I hate to push this back again, but I have to say this: the patch should contain only one idea (I am sometimes guilty of mixing things up, too.); so the 'unmarshal' bit should be split off this patch to keep the issues separate.

        We are almost there, I think.

        Show
        Hiro Asari added a comment - Benoit, Thank you for updating the patch. I hate to push this back again, but I have to say this: the patch should contain only one idea (I am sometimes guilty of mixing things up, too.); so the 'unmarshal' bit should be split off this patch to keep the issues separate. We are almost there, I think.
        Hide
        Benoit Daloze added a comment -

        Sure, I understand. Here it is then.

        It might be worth to mention in the wiki that you really like one single concern per patch though.

        Show
        Benoit Daloze added a comment - Sure, I understand. Here it is then. It might be worth to mention in the wiki that you really like one single concern per patch though.
        Hide
        Hiro Asari added a comment -

        Thanks, Benoit.

        I merged the patch to the master (64375dc) and the 1.6 branch (6348138).

        Show
        Hiro Asari added a comment - Thanks, Benoit. I merged the patch to the master (64375dc) and the 1.6 branch (6348138).

          People

          • Assignee:
            Hiro Asari
            Reporter:
            Benoit Daloze
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: