[Sugar-devel] Github review workflow

Martin Abente martin.abente.lahaye at gmail.com
Wed Apr 10 10:38:31 EDT 2013


Maybe there is a hook that we can use?


On Wed, Apr 10, 2013 at 10:24 AM, Daniel Narvaez <dwnarvaez at gmail.com>wrote:

> Hi Martin,
>
> Thanks for trying it out! I hope more reviewers will follow you :)
>
> I tend to agree about the merge commit, especially for small patch sets.
>
> A pull request is just a branch. If you click the (i) button in the pull
> request green box, you get instructions on how to pull it locally (they can
> probably be shortened a bit by avoiding to use a separate branch). Then
> with interactive rebase you can edit the commit logs and add the acked-by.
>
> If you have a lot of patches the interactive rebase is going to be
> annoying. We could script that part, I would actually be surprised if no
> one has done it already.
>
>
> On 9 April 2013 23:29, Martin Abente <martin.abente.lahaye at gmail.com>wrote:
>
>> Hello:
>>
>> I was testing the workflow today (thanks a lot to manuq for the
>> assistance) and it seems easy enough. The only thing that I didn't like was
>> the merge-commit, IMHO is a little-bit noisy. I know the importance of
>> leaving a trace about who's pushing the change, but a whole commit for that
>> doesn't seem right to me.
>>
>> I got to push manuq changes without the the merge-commit, but it didn't
>> leave any trace of me pushing it, which is something we require. So, does
>> occur to someone a way to get the best from both worlds (IE, a way to have
>> the acked-by) :)?
>>
>> Saludos,
>> Tincho.
>>
>> On Tue, Apr 9, 2013 at 12:55 PM, Manuel Quiñones <manuq at laptop.org>wrote:
>>
>>> 2013/4/8 Manuel Quiñones <manuq at laptop.org>:
>>> > 2013/3/28 Daniel Narvaez <dwnarvaez at gmail.com>:
>>> >> Hi,
>>> >>
>>> >> I started experimenting a bit with github code reviews, with Walter as
>>> >> ginuea pig :) We wasn't too sure about stuff like rebasing, using
>>> >> separate branches etc, so tonight I played a bit myself with creating
>>> >> pull requests.
>>> >>
>>> >> Something like the following might be a decent start for a workflow
>>> >>
>>> >> 1 Create one branch per topic
>>> >
>>> > 0. Fork
>>> >
>>> > I forgot to do it in my first pull-request submission.
>>>
>>> And add a remote for upstream.  Example:
>>>
>>> git remote add upstream https://github.com/sugarlabs/sugar.git
>>>
>>> Which is useful to keep your fork in sync.  This is for the record, we
>>> need to write the documentation of the workflow.
>>>
>>> >>
>>> >> git checkout -b topic1
>>> >>
>>> >> 2 Make one or more commits
>>> >> 3 Push the branch
>>> >>
>>> >> git push origin topic1
>>> >>
>>> >> 4 Submit a pull request for the branch (web UI)
>>> >>
>>> >> 5a The reviewer merges the patch.
>>> >> 5b The reviewer rejects the patch (and closes the request).
>>> >> 5c The reviewer requires changes (and closes the request).
>>> >>
>>> >> If 5c:
>>> >>
>>> >> 6 Make changes using interactive rebase
>>> >> (
>>> http://git-scm.com/book/en/Git-Tools-Rewriting-History#Changing-Multiple-Commit-Messages
>>> )
>>> >>
>>> >> git rebase -i master
>>> >>
>>> >> 7 Push the changes to another remote branch
>>> >>
>>> >> git push origin topic1:topic1-try2
>>> >>
>>> >> 8 Submit the new pull request (web UI)
>>> >>
>>> >>
>>> >> If someone has experience with github suggestions would be welcome.
>>> >> Otherwise I hope Walter will keep being the ginuea pig in this
>>> >> experiment  :)
>>> >>
>>> >> --
>>> >> Daniel Narvaez
>>> >> _______________________________________________
>>> >> Sugar-devel mailing list
>>> >> Sugar-devel at lists.sugarlabs.org
>>> >> http://lists.sugarlabs.org/listinfo/sugar-devel
>>> >
>>> >
>>> >
>>> > --
>>> > .. manuq ..
>>>
>>>
>>>
>>> --
>>> .. manuq ..
>>> _______________________________________________
>>> Sugar-devel mailing list
>>> Sugar-devel at lists.sugarlabs.org
>>> http://lists.sugarlabs.org/listinfo/sugar-devel
>>>
>>
>>
>
>
> --
> Daniel Narvaez
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20130410/597630de/attachment.html>


More information about the Sugar-devel mailing list