<div class="gmail_quote">On Sat, Aug 27, 2011 at 4:56 AM, Antoine Pitrou <span dir="ltr">&lt;<a href="mailto:solipsis@pitrou.net">solipsis@pitrou.net</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Sat, 27 Aug 2011 04:37:21 +0300<br>
Ezio Melotti &lt;<a href="mailto:ezio.melotti@gmail.com">ezio.melotti@gmail.com</a>&gt; wrote:<br>
&gt;<br>
&gt; I&#39;m not sure it&#39;s worth doing an extensive review of the code, a better<br>
&gt; approach might be to require extensive test coverage  (and a review of<br>
&gt; tests).  If the code seems well written, commented, documented (I think<br>
&gt; proper rst documentation is still missing),<br>
<br>
</div>Isn&#39;t this precisely what a review is supposed to assess?<br></blockquote><div><br>This can be done without actually knowing and understanding every single function in the module (I got the impression that someone wants this kind of review, correct me if I&#39;m wrong).<br>
 </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
&gt; We will get familiar with the code once we start contributing<br>
&gt; to it and fixing bugs, as it already happens with most of the other modules.<br>
<br>
</div>I&#39;m not sure it&#39;s a good idea for a module with more than 10000 lines<br>
of C code (and 4000 lines of pure Python code). This is several times<br>
the size of multiprocessing. The C code looks very cleanly written, but<br>
it&#39;s still a big chunk of algorithmically sophisticated code.<br></blockquote><div><br>Even unicodeobject.c is 10k+ lines of C code and I got familiar with (parts of) it just by fixing bugs in specific functions.<br>
I took a look at the regex code and it seems clear, with enough comments and several small functions that are easy to follow and understand.  multiprocessing requires good knowledge of a number of concepts and platform-specific issues that makes it more difficult to understand and maintain (but maybe regex-related concepts seems easier to me because I&#39;m already familiar with them).<br>
<br>I think it would be good to:<br>  1) have some document that explains the general design and main (internal) functions of the module (e.g. a PEP);<br>  2) make a review on rietveld (possibly only of the diff with re, to limit the review to the new code only), so that people can ask questions, discuss and understand the code;<br>
  3) possibly update the document/PEP with the outcome of the rietveld review(s) and/or address the issues discussed (if any);<br>  4) add documentation for the module and the (public) functions in Doc/library (this should be done anyway).<br>
<br>This will ensure that the general quality of the code is good, and when someone actually has to work on the code, there&#39;s enough documentation to make it possible.<br><br>Best Regards,<br>Ezio Melotti<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

<br>
Another &quot;interesting&quot; question is whether it&#39;s easy to port to the PEP<br>
393 string representation, if it gets accepted.<br>
<br>
Regards<br>
<font color="#888888"><br>
Antoine.<br>
</font><div><div></div><br></div></blockquote></div>