Wine patch code review or what to do if patches get stuck?

Igor Tarasov tarasov.igor at gmail.com
Tue Apr 21 02:44:03 CDT 2009


Hi!

The reason of why I write this to you is simple: my patches have stuck
again and I don't know what to do. But since this happening not with
me only and not this happens regularly, I have thought about looking
at the problem a bit wider.

Is there any code review system for patches that are sent to
wine-patches? Or just some developers monitor wine-patches and say
something if they have something to say? Or, how does Alexander make
decisions regarding committing patches?

I've been thinking about some kind of web-based system, like one used
at http://codereview.chromium.org/

It would be nice, if there was sime kind of bot, that would grab all
patches from wine-patches, sort them, make some initial analysis
(maybe, run local test git repository, make various tests, check that
threre are no conflicts, check code style (at least whitespace
changes) etc). Then it could set a flag and report back to the sender.
For instance it would flag "tests fail" and send details back to
sender. Or "whitespace change detected" and send back the quoted diff
parts.

Then, somebody of developers might review it and put his mark/comments
that would go to wine-devel (if necessary), or to sender directly.

There could be simple templates for test reviews for common problems,
and tags, such as

"sure?" that means, that more explanations are needed why that
implementation is correct.
"too hackish" that means that style or code itself is of bad quality.
"whitespace" whitespacechange detected.
"tests required" tests are really required for this patch to go in.
"okay", means that reviewer recommends the patch for commitment.
"wrong", means that patch is totally wrong
..etc

Some of these tags mean that report goes directly to developer, and
patch is marked as obsolete, others make system send some feedback to
wine-dev.

It would take just few clicks for reviewer to get it done. This would
provide good feedback, also this would help the one who makes the
decisions to see that others have already checked that patch and what
they think about that.

Sometimes "no feedback" means that you just dont know - is there
something wrong? Or maybe your patch wos not understood correctly? Or
there are doubts that what you did is right? Or what? It would be
better to have feedback like "are you serious?" than nothing at all :)

P.S: the patches in question are
http://www.winehq.org/pipermail/wine-patches/2009-April/072009.html
and http://www.winehq.org/pipermail/wine-patches/2009-April/072013.html
. They both are soo simple, that I even don't know how to ask of
what's wrong with them :)

-- 
Igor



More information about the wine-devel mailing list