This came up on the linuxwacom-devel list today and I think it warrants further spread through this post.
Different projects have different patch review requirements but the biggest difference is pre-review and post-review. That is, do patches get reviewed before or after they hit the repositories. Not too long ago, the X server employed a post-review process. Everyone with access pushed and bugs would get discovered by those reading commit logs. Patches that ended up on the list were mainly from those that didn't have commit access. Beginning with server 1.8 we introduced a hard review requirement and every patch to make it into the repos ended up on the list, so we switched from post-review to pre-review.
Aside from enforcing that someone gives the formal ACK for a patch, a side-effects is to allow for a passive "drive-by" learning of the code base. Rather than having to explicitly look up commit logs, patches are delivered into one's inbox, outlining where the development currently happens, what it is about and - perhaps most importantly - issues that may have been caused bugs in rejected patches. Ideally that is then archived, so you can link to that discussion later.
The example I linked to from above is automake's INCLUDES versus AM_CPPFLAGS. I wouldn't have know about them if it wasn't for skimming through Gaetan's patches to various X.Org-related projects. That again allowed me to contribute a (in this case admittedly minor) patch to another project. And since that patch ended up on another list, the knowledge can spread on.
Next time when you think of committing directly to a repo, consider sending the patches out. Not just for review, but also to make learning easier for others.
2 comments:
Peter:
Thank you for another excellent post. The idea of pre-review is new to me (embarrassingly). Do you have references for automating pre-reviews with, for example, git? Thank you.
Jackman:
pre-review just means you get someone to look at a patch before you push to your upstream repo. We do it by just generating patches through git-format-patch and git-send-email to a mailing list. That's as automated as it gets for me, there's value in sending it manually. Quite often I see issues in the mail viewer that I didn't spot in git diff and have to go back and re-do the patch :)
Post a Comment