[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] Team branches and gerrit
From:       "Olle E. Johansson" <oej () edvina ! net>
Date:       2015-04-27 12:59:33
Message-ID: D8DE9638-8ADF-4F95-A663-C594F671459B () edvina ! net
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On 27 Apr 2015, at 14:55, Russell Bryant <russell@russellbryant.net> wrote:

> On Mon, Apr 27, 2015 at 8:20 AM, Olle E. Johansson <oej@edvina.net> wrote:
> 
> On 24 Apr 2015, at 15:42, Russell Bryant <russell@russellbryant.net> wrote:
> 
> > On Fri, Apr 24, 2015 at 8:31 AM, Joshua Colp <jcolp@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
> 
THat makes sense.

/O


[Attachment #5 (unknown)]

<html><head><meta http-equiv="Content-Type" content="text/html \
charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space;"><br><div><div>On 27 Apr 2015, at \
14:55, Russell Bryant &lt;<a \
href="mailto:russell@russellbryant.net">russell@russellbryant.net</a>&gt; \
wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div \
dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 27, 2015 at \
8:20 AM, Olle E. Johansson <span dir="ltr">&lt;<a href="mailto:oej@edvina.net" \
target="_blank">oej@edvina.net</a>&gt;</span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
style="word-wrap:break-word"><div><div class="h5"><br><div><div>On 24 Apr 2015, at \
15:42, Russell Bryant &lt;<a href="mailto:russell@russellbryant.net" \
target="_blank">russell@russellbryant.net</a>&gt; wrote:</div><br><blockquote \
type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, \
Apr 24, 2015 at 8:31 AM, Joshua Colp <span dir="ltr">&lt;<a \
href="mailto:jcolp@digium.com" target="_blank">jcolp@digium.com</a>&gt;</span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span>Olle \
E. Johansson wrote:<br> <blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
 Playing around following Matt's wiki page on gerrit usage, I created<br>
a team branch and did two commits. When pushing it with "git review<br>
{branch}" only the last commit shows up.<br>
<br>
Is that the way it's supposed to be? I thought the whole branch was<br>
the review subject, not just a single commit.<br>
</blockquote>
<br></span>
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.<br></blockquote><div><br></div><div>Gerrit can also work on a patch series, \
and tracks dependencies between those \
patches.</div></div></div></div></blockquote><br></div></div></div><div>Define "patch \
series" - how do you commit a series of patches for \
review?</div></div></blockquote><div><br></div><div>In some cases, it makes sense for \
a feature or fix to be proposed as multiple changes in a series.&nbsp; I feel that it \
makes things easier to review and understand.</div><div><br></div><div>As a \
completely realistic example, let's assume we want to add support for toaster control \
to chan_pjsip.&nbsp; That could be submitted as a series of 3 \
commits:</div><div><br></div><div>1 - ast_toaster: Add core modular API for toaster \
control.</div><div>2 - toastip - Add ast_toaster backend for the toastip \
protocol.</div><div>3 - pjsip_toast - Add chan_pjsip integration with the ast_toaster \
API.&nbsp;</div><div><br></div><div>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.</div><div><br></div><div>When it comes to what that actually looks like \
it git.</div><div><br></div><div>Create a local branch for a \
feature:</div><div><br></div><div>&nbsp; $ cd asterisk-git</div><div>&nbsp; $ git \
checkout -b example-patch-series origin/master</div><div><br></div><div>Commit \
1:</div><div><br></div><div>&nbsp; $ echo "some text" &gt; foo</div><div>&nbsp; $ git \
add foo</div><div>&nbsp; $ git commit -m "Add foo"</div><div><br></div><div>Commit \
2:</div><div><br></div><div>&nbsp; $ echo "some more text" &gt;&gt; \
foo</div><div>&nbsp; $ git commit -a -m "Add more text to \
foo"</div><div><br></div><div>We now have 2 commits on top of the master \
branch:</div><div><br></div><div><div>&nbsp; $ git log --oneline \
origin/master..HEAD</div><div>&nbsp; 5a53b4f Add more text to foo.</div><div>&nbsp; \
4bf86fa Add foo.</div></div><div><br></div><div>Post series of 2 commits for \
review:</div><div><br></div><div><div>&nbsp; $ git review</div><div>&nbsp; You are \
about to submit multiple commits. This is expected if you are</div><div>&nbsp; \
submitting a commit that is dependent on one or more in-review</div><div>&nbsp; \
commits. Otherwise you should consider squashing your changes into \
one</div><div>&nbsp; commit before submitting.</div><div><br></div><div>&nbsp; The \
outstanding commits are:</div><div><br></div><div>&nbsp; 5a53b4f (HEAD, \
example-patch-series) Add more text to foo.</div><div>&nbsp; 4bf86fa Add \
foo.</div><div><br></div><div>&nbsp; Do you really want to submit the above \
commits?</div><div>&nbsp; Type 'yes' to confirm, other to \
cancel:&nbsp;</div></div><div><br></div><div>(You would type 'yes' here because \
you're submitting 2 changes for review \
intentionally.)</div><div><br></div><div>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:</div><div><br></div><div>&nbsp;&nbsp;<a \
href="https://wiki.openstack.org/wiki/GitCommitMessages">https://wiki.openstack.org/wiki/GitCommitMessages</a></div><div><br></div></div></div></div></blockquote>THat \
makes sense.</div><div><br></div><div>/O</div><br></body></html>



-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic