[darcs-devel] review policy revisited

Guillaume Hoffmann guillaumh at gmail.com
Tue Feb 22 16:51:30 UTC 2011


I was thinking about this thread again. Maybe we already have what we
want, but we don't dare acting as we should to make it work?

> - "my non-activity should not beat your activity" -- others not doing
>  anything should not block an active contributor

We have this: core members can push to screened when they feel their
patches are mature enough to go, without waiting for anyone else's
approval. Of course moving patches from screened to darcs.net requires
other people's reviews, but at least everything that is in screened
will stay (maybe rollbacked).

> - it enables "silent" reviewing -- for me, personally, formulating
>  feedback is a significant distraction from actually reading and
>  understanding code; I like to be able to approve by silence
> - the review backlog is a witness of the review process not working very
>  well: patches are being blocked in review for a long time, leading to
>  all sorts of complications... review and code discussion in general is
>  best done while the author has the code in fresh memory

Saying "ok, these patches seem to do what's written on them and nobody
complained about them since X weeks they are in screened, so I'm
pushing them" should be fine. This is more or less what I did for
Ganesh's patches on patch code refactoring.

This is obviously more costly in time than the silent reviewing
approach (the compile+testing process is really the most costly on my
machine, however), but it would give me more confidence to see patches
accepted with one-liner messages than no messages at all. This is
because I don't trust myself a lot. I don't think I want my patches to
automatically go in without at least a guarantee that someone else
gave a look.

I also think reviews should be substantially more compact than the
patch itself, when the patch is big. If I want to look at the patch, I
just look at the patch.

guillaume


More information about the darcs-devel mailing list