[Sugar-devel] Acked-by vs Reviewed-by

[Daniel Narvaez]

On 4 April 2013 19:02, Manuel Quiñones <manuq at laptop.org> wrote:

> 2013/4/3 James Cameron <quozl at laptop.org>:
> > That's really up to the maintainer.  If the maintainer pushes the
> > patch, then Acked-by may be inferred.
> >
> > In general these procedures scale well to large numbers of
> > maintainers, contributors, and reviewers.  I'm not sure they remain
> > appropriate for Sugar given the size of the community at the moment.
>
> In the case of a pull request workflow, the merger (author of the
> merge commit) has the same meaning as the Acked-by.  So if the signing
> was useful to know who reviewed/acked, the author of the merge is the
> same in the pull-request.  I just merged a pull request from Daniel
> Narvaez
>
> Here is his commit:
>
>
> https://github.com/dnarvaez/sugar-toolkit-gtk3/commit/db448c4eea41a5661838c3a4f3788457fe28ac77
>
> And my merge:
>
>
> https://github.com/sugarlabs/sugar-toolkit-gtk3/commit/a776c72d46ac09a2791e75a7edbcbc534b158ab5
>
> Anyways I added "Acked-by" to the merge commit message to not break
> the current rules.  But we can see the duplication.
>
> Merging can be done by the command line, not only by the github UI
>
> https://help.github.com/articles/merging-a-pull-request
>
> And I think we should recommend doing the merge locally for testing
> before merge.  'git merge' automatically opens the editor since git
> git1.7.9.6
>

I agree about recommending to merge locally.

I wonder if we should have merge commits though, or if we should rebase.
They do add a bit of noise to history but they also add some information.
Maybe it depends, as argued here

http://mislav.uniqpath.com/2013/02/merge-vs-rebase/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20130404/f720983f/attachment.html>