Code review and commits to the official repository

[Luke Yelavich]

On Tue, Aug 10, 2010 at 11:08:37PM EST, Hynek Hanke wrote:
> 
> Hello all,
> 
> in the spirit of the Cooperation guidelines
>     ( http://www.freebsoft.org/cooperation ),
> it would be good to establish proper code review
> from now on. Here is a proposal.
> 
> A piece of code (a patch on the mailing list, a branch
> from a private Git repository etc.) cannot be submitted
> to the repository by the original developer himself,
> but must be first submitted for review. The reviewer
> will push it to the official Git repository. This is the same
> for Brailcom and other developers.

I agree with this.

> We propose the code reviewers can be (it they like to):
> 
>     Christopher Brannon
>     Luke Yelavich
>     Hynek Hanke
>     Tomas Cerha

Thank you very much for the offer, and expressing confidence in me to do a good 
job. I accept.

> If anyone has a proposal for another person, please let
> us know. Of course we will welcome if the list is extended
> in the future.
> 
> These persons will have commit access to the official
> Git repository, but should not generally use it to commit
> their own code.
> 
> The reviewers will then be responsible for the code they
> commit.

I agree with Chris, I think the reviewer should sign off the commit they 
review, to clearly indicate the approval of the patch.

> There is also the question how to coordinate. I think it
> would be best if we take pieces of code submitted through
> this mailing list one by one as we find time to review them
> and always just say: ,,I'm looking into that...'', to prevent
> two persons working on the same.

SOunds good.

> We did internal review for 0.7.0, but I'd also highly welcome
> if somebody could check my patches so far for the 0.7.1
> release, which already are in master, to make sure I didn't
> overlook something while implementing the new improvements.

I'll have a look when I get a chance.

Luke