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

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.

Barry has yelled at many people, including me, about // comments and declarations in the middle of a block.

Being able to push means LISTENING when these things happen on the mailing list.

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.

There are essentially no comments on this massive push. I was just reading it, getting mad. Jed mailed first.

Pushes need enough comments that I can see exactly what functionality is being added and how it works.

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

time will be wasted.

Matt

This is non-portable:

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.