[FFmpeg-devel] [RFC] replace some static with asm_visibility or?so

Reimar Döffinger Reimar.Doeffinger
Wed Jan 30 13:26:46 CET 2008


Hello,
On Wed, Jan 30, 2008 at 01:13:04PM +0100, Michael Niedermayer wrote:
> And please dont forget reviewing is about checking that the code and its
> design is (near) optimal, much more than it is about checking that
> indention and doxygen matches the rules. So skiping that would definitly
> not do any good. It would be like spellchecking a document whos content
> makes no sense.

Yes, but I'd also like to say that e.g. a missing doxygen document means
that in order to review the code I basically have to invest additional
time to find before I understand what the function does, I have to
"reverse engineer" from the remaining code what the function _should_ do
and then I still have to guess if the function or the using code is
wrong.
This is a very good reason to check these kind of things before, simply
because it means less effort to the reviewer.
And as much as I hate it whenever I submit a more complex patch myself,
even for FFmpeg reviewers are the rarer resource (though thanks to
Michael the situation is not as desperate as for MPlayer).
But anyway, if something bothers you, say it directly as a response to
such an email, general comments are just not as useful by far, due to
lacking context and lacking a possibility to give or refute _specific_
suggestions for improvement.

Greetings,
Reimar Doeffinger




More information about the ffmpeg-devel mailing list