[Sugar-devel] Github review workflow

Daniel Narvaez dwnarvaez at gmail.com
Wed Apr 10 09:24:43 EDT 2013


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/6d66db83/attachment.html>


More information about the Sugar-devel mailing list