[MPlayer-cvslog] r27849 - trunk/libvo/x11_common.c

[Reimar Döffinger]

On Fri, Oct 31, 2008 at 04:08:39PM +0200, Uoti Urpala wrote:
> On Fri, 2008-10-31 at 12:08 +0100, Reimar Döffinger wrote:
> >  I just want a commit
> > message that give at least some idea of the point of it, which hopefully
> > the committer can answer.
> 
> I agree that the commit message did not explain the rationale clearly,
> but I'm not sure what you mean by the "which hopefully the committer can
> answer" part. Do you think it's still unclear whether a justified
> rationale exists? IMO the fact that there are documented cases where
> backing store is harmful and no indication of it being useful does
> justify the commit.

My real problem is: the commit message is utter crap, and that in a case
of an obviously controversial change!
First the message says: "Also, it is mandatory for Xserver 1.5.x"
Huh? That was not mentioned in any of the discussion, what does this
mean? Or was it supposed to say: "Fixes horrible performance with
Xserver 1.5.x in some cases"?
There is also: "and will be removed from Xserver 1.6 anyhow"
Does that mean the previous code would no longer compile with 1.6?
Or does it mean that the flag would be silently ignored (in which case
it could be argued that it is not removed but actually a performance
regression that 1.5 had is being fixed)?
I think ifdeffery is overkill since we have no indication it matters on
older X versions or alternative X implementations but if for example in
one year someone comes and says "hey, on my OS xyz there is a problem
since that flag was removed, just add it in again" the commit message
has enough info so we can say "well, we will need a special case for
this and that", and no the mailing list link is no good because
a) there is just too much stuff in there, some theoretical discussion,
some wrong stuff etc.
b) it might not work anymore e.g. in one year's time.

Greetings,
Reimar Döffinger