[petsc-dev] Patches like this make me want to cry

[Jed Brown]

On Thu, Jun 28, 2012 at 2:24 PM, Paul Mullowney <paulm at txcorp.com> wrote:

> VecTransplantPlaceArray
>
>  http://petsc.cs.iit.edu/petsc/**petsc-dev/rev/d2f118b395b2<http://petsc.cs.iit.edu/petsc/petsc-dev/rev/d2f118b395b2>
>>
>> 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.
>>
>>
>>  Why is the non-portable?


It assumes C99 to begin with.


> I asked for your input on this 2 days ago?


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.

Also, you didn't provide the whole patch series to comment on, just some
bits of code.


>
>  This is non-portable:
>>
>> http://petsc.cs.iit.edu/petsc/**petsc-dev/rev/66ca8db0d5f8<http://petsc.cs.iit.edu/petsc/petsc-dev/rev/66ca8db0d5f8>
>>
>>
>> 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.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120628/2d388b56/attachment.html>