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

Randy Abernethy commented on THRIFT-2926:
-----------------------------------------

Hey [~radekg],

The tests for this patch are presently writting and reading from the protocol's 
internal buffer. This is fine, but usually something you can skip if you have 
end ot end tests working. You may note all of the other tests are end to end 
(client uses generated IDL code, serializes with proto, sends to transport, 
transmits to server, and back). This is really a requirement for any protocol, 
we need to have tests that show it working outright to know it works on initial 
commit and to avoid regression. After building a test web client and node 
server to actually use TBinaryProtocol in such a setting I discovered it never 
writes to the transport (for reference, the write is done in writeMessageEnd() 
in TJSONProtocol). 

I really wanted to get this in prior to jumping onto the several node patches 
waiting  but it is not ready. If you can get the patch to lint clean with 
JSHint and run the test-async.js test suite clean I'll revisit  (see: 
testws.html for an example).

Best,
Randy


P.S. [~andrewdeandrade], I will get the node/compact double patch in next and 
then start on your list.

> JavaScript: binary protocol implementation
> ------------------------------------------
>
>                 Key: THRIFT-2926
>                 URL: https://issues.apache.org/jira/browse/THRIFT-2926
>             Project: Thrift
>          Issue Type: Sub-task
>          Components: JavaScript - Library
>    Affects Versions: 0.9.2
>            Reporter: Radoslaw Gruchalski
>            Assignee: Randy Abernethy
>              Labels: features, javascript
>
> I have implemented BinaryProtocol for the JS library. Implementation provided 
> with unit tests. Looking for feedback.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to