Re: Lessons from commit fest

"Stephen Frost" <sfrost(at)snowman(dot)net> writes:

> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> One problem with this fest was that a whole lot of the patches *weren't*
>> trivial; if they had been they'd not have gotten put off till 8.4. So
>> there weren't that many that I wanted to let go in without looking at
>> them. I guess that's just another way in which the 8.3 schedule problem
>> came home to roost during this fest.
>
> That's an issue I ran into when looking through the patches as well. I
> can help review trivial patches but I don't generally have the
> bandwidth to review alot of the pretty heavy patches which were in the
> March commitfest. It took me a while to get going on the commitfest too
> though, in part because I wasn't really confident I knew the right
> places to look and the right things to do. I'm still not 100% sure,
> honestly. Do we have guidelines up somewhere about reviewing, specific
> to the process rather than about how to submit patches?

I don't think we know what a "typical review" really looks like. But basically
what worked for me in the (relatively trivial) patches I've looked at was
considering "if this was mine, what would I consider doing next?" I made a
bullet point list of things I would consider changing or think are missing.

One thing I found is that I'm not sure what to do if I don't find any changes
to propose. I tend to assume that means I just don't understand the code well
enough and end up just not posting anything.

>> Perhaps it would be useful to try to rate pending patches by difficulty?
>
> I think alot of times this can be determined pretty quickly, and I'd
> hate to have a situation where patch submitters are complaining about
> "you rated my patch harder than his and that's not fair" kind of things.

One thing which is conventional on linux-kernel is to include the output of
diffstat when you post the patch. That helps people to get an idea of whether
their favourite area of the code is being whacked around and it warrants a
look. It also helps get a quick overview of what to expect as you start to
read the patch proper.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!