On Thu, Jun 28, 2012 at 4:33 PM, Jed Brown <span dir="ltr"><<a href="mailto:jedbrown@mcs.anl.gov" target="_blank">jedbrown@mcs.anl.gov</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote">On Thu, Jun 28, 2012 at 2:24 PM, Paul Mullowney <span dir="ltr"><<a href="mailto:paulm@txcorp.com" target="_blank">paulm@txcorp.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

VecTransplantPlaceArray<div class="im"><div><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://petsc.cs.iit.edu/petsc/petsc-dev/rev/d2f118b395b2" target="_blank">http://petsc.cs.iit.edu/petsc/<u></u>petsc-dev/rev/d2f118b395b2</a><br>
<br>
This thing is way too big to review, includes huge swaths of commented-out code, breaks coding conventions and portability, and introduces strange new APIs (like VecTransplantPlaceArray) that haven't really been explained and seem to produce questionable semantics.<br>


<br>
<br>
</blockquote></div></div>
Why is the non-portable?</blockquote><div><br></div><div>It assumes C99 to begin with.</div></div></blockquote><div><br></div><div>Barry has yelled at many people, including me, about // comments and declarations in the middle of a block.</div>
<div>Being able to push means LISTENING when these things happen on the mailing list.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I asked for your input on this 2 days ago?</blockquote>
<div><br></div><div>Sorry, I'm at a conference, finishing a proposal, and trying to make progress on my research. I was hoping someone else would comment because I think it's leaking implementation details.</div>
<div>
<br></div><div>Also, you didn't provide the whole patch series to comment on, just some bits of code.</div></div></blockquote><div><br></div><div>There are essentially no comments on this massive push. I was just reading it, getting mad. Jed mailed first.</div>
<div>Pushes need enough comments that I can see exactly what functionality is being added and how it works.</div><div>You will not be the maintainer of this functionality, we will. If we can't do it, it will be thrown out, and all your</div>
<div>time will be wasted.</div><div><br></div><div>   Matt</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div><div><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This is non-portable:<br>
<br>
<a href="http://petsc.cs.iit.edu/petsc/petsc-dev/rev/66ca8db0d5f8" target="_blank">http://petsc.cs.iit.edu/petsc/<u></u>petsc-dev/rev/66ca8db0d5f8</a><br>
<br>
<br>
Can we please institute some sort of policy on patch quality/reviewability? This one patch is going to take a significant amount of fix-up (not made easier by the several merges since) and/or generate several build failures and user inconvenience (petsc-maints). No doubt the functionality is important, but we just don't have time to fix these things line-by-line after they are pushed.<br>


</blockquote>
<br>
</div></div></blockquote></div></div><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead.<br>
-- Norbert Wiener<br>