[asterisk-dev] DAHDI migration to Git

[Russell Bryant]

On Thu, Dec 6, 2012 at 11:37 AM, Shaun Ruffell <sruffell at digium.com> wrote:

> As an aside, the one piece of this puzzle that is less than ideal is
> code review.
>
>   - Reviewboard didn't allow me to present a change set as a series
>     of commits easily but rather wanted all the changes squashed
>     into a single patch on top of subversion.
>
>   - Crucible didn't allow the commit messages to be reviewed along
>     with the code change itself.
>
>   - Gerrit is still in my queue to demo to see if it will solve the
>     issue so I don't have a comment there.
>
>   - Reviews by email (which I have a strong preference for for many
>     of the same reasons it works so well for the Linux kernel) may
>     have legal issues for a potentially dual-licensed project like
>     Asterisk / DAHDI. One passing thought I had about this is
>     setting up a new mailing list for DAHDI development which can be
>     copied on 'git send-email'. But when it comes time to actually
>     merge the code it still must be via
>     issues.asterisk.org/git.asterisk.org/signed tag. But I think
>     lawyers are going to need to get involved to see if that will
>     provide sufficient protection to Asterisk as a project.
>

I've been using gerrit a lot lately and I like it quite a bit.  Some
interesting notes:

1) It does support posting a patch series.  The UI isn't perfect for it,
but it's there.  I expect that to be improved in gerrit in the future given
the git-centric nature of gerrit and the importance of patch series for
projects using git.

Here is one example:
https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/nova-compute-cells,n,z

If you go into a specific patch, you will see a "Dependencies" section,
where you can see the patch(es) that come before and/or after the one
you're looking at.

2) The commit message is reviewed just like the source changes in a patch.
 Again, take a look at any patch above as an example.

3) Gerrit has some built-in CLA checking logic that can be hooked into.
review.openstack.org uses it.  You must have a CLA to be able to push
changes there, which is the only path for getting patches in.

-- 
Russell Bryant
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121206/fdf17678/attachment.htm>