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

Chris Douglas edited comment on HADOOP-10389 at 6/12/14 8:14 PM:
-----------------------------------------------------------------

[~wheat9], can you be more specific about the code you've found difficult to 
review and audit, and what could be done to improve its readability? A treatise 
on buffer overflows and memory leaks will not eliminate them; if Colin were to 
produce it, how would it help you? Comparing a hypothetical implementation 
against a partial one by arguing about language features is very indirect. As 
an example, [~decster]'s questions showed that he had made an effort to 
understand the code, and Colin's answers provided some insight into the 
approach. If the code is so impenetrable that review is truly impossible, 
please consider demonstrating the virtues of C++11 in a competing branch.

[~cmccabe], I looked at the patch attached to this JIRA. Some of it was 
familiar and/or requires cursory review (makefiles, a queue impl from 1994 
\*BSD, vint utility methods), but other files (common/string.*, a splay tree 
impl from...?, protoc-gen-hrpc.cc) are less obvious, and you didn't explain 
them until asked. If you can help others follow your progress in the branch, 
they won't need to ask the "right" questions to understand your approach. This 
can be in design docs[1], thorough comments[2], and calling out which files are 
new code (requiring the rigorous audit that [~wheat9] rightly expects), and 
what has been working for 20+ years. You can solicit better questions.

[1] Is there a design doc for this? I looked on HADOOP-10388, but didn't find 
one.
[2] For example, one of the few comments in this patch was for 
{{hrpc_conn_read_alloc}}, which explained esoteric details of the interaction 
with libuv.


was (Author: chris.douglas):
[~wheat9], can you be more specific about the code you've found difficult to 
review and audit, and what could be done to improve its readability? A treatise 
on buffer overflows and memory leaks will not eliminate them; if Colin were to 
produce it, how would it help you? Comparing a hypothetical implementation 
against a partial one by arguing about language features is very indirect. As 
an example, [~decster]'s questions showed that he had made an effort to 
understand the code, and Colin's answers provided some insight into the 
approach. If the code is so impenetrable that review is truly impossible, 
please consider demonstrating the virtues of C++11 in a competing branch.

[~cmccabe], I looked at the patch attached to this JIRA. Some of it was 
familiar and/or requires cursory review (makefiles, a queue impl from 1994 
*BSD, vint utility methods), but other files (common/string.*, a splay tree 
impl from...?, protoc-gen-hrpc.cc) are less obvious, and you didn't explain 
them until asked. If you can help others follow your progress in the branch, 
they won't need to ask the "right" questions to understand your approach. This 
can be in design docs[1], thorough comments[2], and calling out which files are 
new code (requiring the rigorous audit that [~wheat9] rightly expects), and 
what has been working for 20+ years. You can solicit better questions.

[1] Is there a design doc for this? I looked on HADOOP-10388, but didn't find 
one.
[2] For example, one of the few comments in this patch was for 
{{hrpc_conn_read_alloc}}, which explained esoteric details of the interaction 
with libuv.

> Native RPCv9 client
> -------------------
>
>                 Key: HADOOP-10389
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10389
>             Project: Hadoop Common
>          Issue Type: Sub-task
>    Affects Versions: HADOOP-10388
>            Reporter: Binglin Chang
>            Assignee: Colin Patrick McCabe
>         Attachments: HADOOP-10388.001.patch, HADOOP-10389.002.patch, 
> HADOOP-10389.004.patch, HADOOP-10389.005.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to