[ 
https://issues.apache.org/jira/browse/THRIFT-276?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12694756#action_12694756
 ] 

Kevin Clark commented on THRIFT-276:
------------------------------------

This generally looks fine, and everything passes. I've no objections, just 
clarifying questions:

There's still a bunch of classes in transport.rb and protocol.rb. They look 
like reasonable things to keep together (Sockets mostly in transport, and 
errors). Was that the idea?

The 'encoding:' lines are for 1.9?

We talked about this a little bit in IRC, but the BinaryProtocolAccelerated 
name was requested by David for "consistency with php and python". But python's 
file is called fastbinary.c, so I'm not sure what the deal is. Maybe the Python 
_classes_ were BPA. Anyway, I'm not outright against the change, but I'd like 
David's input on it.

Other than that, I'm +1. Good work.


> Ruby libraries should have one class per file
> ---------------------------------------------
>
>                 Key: THRIFT-276
>                 URL: https://issues.apache.org/jira/browse/THRIFT-276
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (Ruby)
>    Affects Versions: 0.1
>            Reporter: Bryan Duxbury
>            Assignee: Michael Stockton
>            Priority: Trivial
>         Attachments: thrift-276-v2.patch, thrift-276-v3.patch, 
> thrift-276.patch
>
>
> There's no reason for so many of our classes to be lumped into the same file. 
> For instance, transport.rb contains 9 classes. They may be short, but 
> organizationally, it's superior to have separate class files. Of course, some 
> files may contain more than one class per file as appropriate - things like a 
> protocol and its factory, for instance, are perfectly acceptable to group.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to