[CinCV TNG] [PATCH] Fix copy and paste error in cwindowgui.C

Johannes Sixt j6t at kdbg.org
Mon Apr 6 00:30:22 CEST 2015


Am 05.04.2015 um 22:05 schrieb Nicola Ferralis:
> Rather than the merit of the patch, you seem to dislike the process that
> led to it.

No, that process does not matter. The justification matters.

> I am not really sure what's wrong with "While staring at the
> code, I noticed this duplicated message...".

Oh, that way of fixing code is not wrong. But when you see something 
questionable, that does not mean it is wrong. If it were wrong, most 
likely, it would have been noticed already.

When I see a questionable construct, I usually form a theory how it can 
be used to trigger a bug, then test the theory (that it really triggers 
a bug), then fix the bug. This immediately leads to a suitable text for 
a justification in the commit message (which would include the 
reproduction recipe).

Such a bug fix patch is much more welcome than a mere cosmetic code 
change, particularly in a complex code base as Cinelerra's, which has 
many more serious problems than cosmetic ones.

Sometimes, I cannot verify my theory (I cannot produce a bug via the 
questionable construct). Then the construct is not worth fixing.

In this case, you would have been unable to trigger a bug. Due to the 
lack of other important reasons, you should stash it away and perhaps 
resend when you have better reasons.

Please keep in mind that reviewers' time is precious. Do not keep them 
busy with trivial stuff.

The preferred way to work is:

You concentrate on a topic that you want to bring to completion. You 
create a series of patches, mend them until they are perfect by your 
standards (which hopefully are at least as high as the project's 
standards), and *then* you present the whole series at once for review.

The trivial patch that we are discussing here *can* be one of the 
preparatory patches of such a series, *if* it is in the vicinity of code 
that is touched by the later patches.

-- Hannes



More information about the Cinelerra mailing list