[asterisk-dev] Team branches and gerrit

[Russell Bryant]

On Mon, Apr 27, 2015 at 8:20 AM, Olle E. Johansson <oej at edvina.net> wrote:

>
> On 24 Apr 2015, at 15:42, Russell Bryant <russell at russellbryant.net>
> wrote:
>
> On Fri, Apr 24, 2015 at 8:31 AM, Joshua Colp <jcolp at digium.com> wrote:
>
>> Olle E. Johansson wrote:
>>
>>> Playing around following Matt's wiki page on gerrit usage, I created
>>> a team branch and did two commits. When pushing it with "git review
>>> {branch}" only the last commit shows up.
>>>
>>> Is that the way it's supposed to be? I thought the whole branch was
>>> the review subject, not just a single commit.
>>>
>>
>> Gerrit works on a single commit (what it refers to as a patch set) that
>> you want included into a specific branch. As a result you need to squash
>> all commits into a single one, and if review feedback warrants further
>> changes they also need to be squashed back into a single commit with the
>> original changes. The single commit you post for review is what is reviewed
>> and merged into the branch.
>>
>
> Gerrit can also work on a patch series, and tracks dependencies between
> those patches.
>
>
> Define "patch series" - how do you commit a series of patches for review?
>

In some cases, it makes sense for a feature or fix to be proposed as
multiple changes in a series.  I feel that it makes things easier to review
and understand.

As a completely realistic example, let's assume we want to add support for
toaster control to chan_pjsip.  That could be submitted as a series of 3
commits:

1 - ast_toaster: Add core modular API for toaster control.
2 - toastip - Add ast_toaster backend for the toastip protocol.
3 - pjsip_toast - Add chan_pjsip integration with the ast_toaster API.

Those *could* all be one commit, but it's really the development of a
feature in 3 logical chunks, so breaking it up like this is an alternative.

When it comes to what that actually looks like it git.

Create a local branch for a feature:

  $ cd asterisk-git
  $ git checkout -b example-patch-series origin/master

Commit 1:

  $ echo "some text" > foo
  $ git add foo
  $ git commit -m "Add foo"

Commit 2:

  $ echo "some more text" >> foo
  $ git commit -a -m "Add more text to foo"

We now have 2 commits on top of the master branch:

  $ git log --oneline origin/master..HEAD
  5a53b4f Add more text to foo.
  4bf86fa Add foo.

Post series of 2 commits for review:

  $ git review
  You are about to submit multiple commits. This is expected if you are
  submitting a commit that is dependent on one or more in-review
  commits. Otherwise you should consider squashing your changes into one
  commit before submitting.

  The outstanding commits are:

  5a53b4f (HEAD, example-patch-series) Add more text to foo.
  4bf86fa Add foo.

  Do you really want to submit the above commits?
  Type 'yes' to confirm, other to cancel:

(You would type 'yes' here because you're submitting 2 changes for review
intentionally.)

Here's a wiki page that I think does a nice job laying out some reasoning
behind best practices for breaking up changes into a series of commits if
anyone is interested:

  https://wiki.openstack.org/wiki/GitCommitMessages

-- 
Russell Bryant
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150427/0ddfd3c5/attachment.html>