[sldev] SLDev Viewer--code review

[Carlo Wood]

On Wed, May 06, 2009 at 01:32:44PM -0400, Mike Monkowski wrote:
> Or would it just be a waste of time?  Does the mention of "voice chat" 
> doom it from its inception?

Main problems for me: I can never, ever, on Thurdays :(
I don't have working voice, and even if I did - that is too
much 'RL' to be mixed with SL.

Aside from that... as said before: it is MORE work to
review a patch than to write one, simply because so
much information needed to check the correctness of
a patch is not in the diff itself.

Therefore each patch contributor should VERY hard try
to be SURE that their patch is correct, and take all
responsibility for that. Basically, if someone commits
a patch that breaks things, the commit rights should
be taken away.

The whole review stuff could be seen to help committers
find that last error they overlooked, but likely none
of the reviewers will see what is wrong by looking at
a diff and hearing the reasoning of the patch writer
anyway... So yes, it's a waste of time.

It would be more productive to have people discuss
concepts (It does this, and I want it to do that,
and this is basically how I want to achieve it)
PRIOR to writing a patch and discuss it then.

Also, people should be able to ask technical questions
WHILE working on the patch and get fast feedback.
[I'll post an example of such question in my next post]

And again, to motivate committers to take their work
seriously, there should be some kind of punishment
when they commit something that is simply broken.

It is better for someone to ask "I am assuming that
this and that class is the ONLY class that keeps
a pointer to this and that object (my patch relies
on that), but is it garanteed, also in the future?"
before committing a patch; then so just write it,
produce silently a diff and hope that reviewers
not only know the answer to the first question but
actually see that the diff is written assuming
said pre-condition.

-- 
Carlo Wood <carlo at alinoe.com>