[Sugar-devel] Usage of Signed-off-by / Acked-by - Re: Recent movement on Sugar master repository

[Gonzalo Odiard]

On Wed, Apr 3, 2013 at 1:27 PM, Daniel Narvaez <dwnarvaez at gmail.com> wrote:

> Perhaps you can explain exactly what Signed-off-by adds compared to the
> author field in tnormal cases? I know it's not much work, but it adds one
> more thing we need to request to patch submitters. I tend to think the
> simpler is the process the more patches we will get :) Or at least I
> personally get really annoyed by bureaucracy when submitting code.
>
>
Usually is redundant, but there are cases where can add value, like when
there are more than one author
or when the author sent a .diff instead of a .patch.


> About Reviewed-by, if you have say a 10 patches set, is one-by-one
> interactive rebase the only way to add the tag to all the patches? I'm not
> too attached to the one click pull request merging from the web UI...
> pulling and pushing on the command line is not too complicated. Though if
> you have to modify patches one by one it starts to get annoying...
>
>
True. Modify every patch can be a pain, specially if we change the way we
are working and move to use github web tools.
I am not sure what is the best to do here. Web interface makes a little
more difficult to see this header info.
(Personally, I feel better in the command line yet)

Gonzalo


>
>
> On 3 April 2013 15:00, Gonzalo Odiard <gonzalo at laptop.org> wrote:
>
>> I think we should keep using the Signed-off-by, Acked-by, Reviewed-by
>> signatures, because do not add too much work,
>> make clear the responsibilities, and recognize important work done
>> as the review, needed by the project.
>>
>> Gonzalo
>>
>>
>> On Tue, Apr 2, 2013 at 2:41 PM, Manuel Quiñones <manuq at laptop.org> wrote:
>>
>>> As we are revisiting all the review process, can we revisit the usage
>>> of signing the patches?  What are they useful for?
>>>
>>> 2013/4/2 Daniel Narvaez <dwnarvaez at gmail.com>:
>>> > So I'm not sure these are worth the overhead. Especially for  signing
>>> it
>>> > doesn't seem to be very useful for us, if not in some special cases,
>>> and it
>>> > adds one more thing we need to require from contributors... I'd rather
>>> bug
>>> > them about improving the code then about using a tag of dubious
>>> utility.
>>> >
>>> > Just my feeling really, I'm happy to keep using the tags if we think
>>> they
>>> > are necessary. Just trying to streamline things a bit if possible.
>>>
>>> Yes, I agree with you Daniel.  I think we need the point of view of
>>> other devs, specially from those who are in the project prior than us.
>>>
>>> >
>>> > On 2 April 2013 19:25, Manuel Quiñones <manuq at laptop.org> wrote:
>>> >>
>>> >> Hi Daniel,
>>> >>
>>> >> 2013/4/2 Daniel Narvaez <dwnarvaez at gmail.com>:
>>> >> > Hey,
>>> >> >
>>> >> > sorry, I forgot about those. Though I think it's worth to think if
>>> we
>>> >> > really
>>> >> > need them while we are reworking reviews a bit.
>>> >> >
>>> >> > When is signed-off-by useful in sugar?
>>> >>
>>> >> I think we just have been following other projects like the linux
>>> >> kernel.  In many projects this means "I certify that my code is
>>> >> compatible with the license of this project".  For me, it is useful
>>> >> only if the commiter is another user, to track the real author.
>>> >>
>>> >> http://wiki.sugarlabs.org/go/Development_Team/Code_Review
>>> >>
>>> >> > How is reviewed-by applied? Is it the reviewer which rebase and add
>>> them
>>> >> > before pushing?
>>> >>
>>> >> Yes we have been amending the patches before pushing, all manual work.
>>> >>  For me, the Acked-by signing has been useful to track who reviewed
>>> >> what.  But maybe not a big deal?
>>> >>
>>> >> So there are projects that do signing, and projects that don't.  I
>>> >> searched quickly for a way to add Acked-by when github merges a pull
>>> >> request, but haven't found anything.
>>> >>
>>> >> > On 2 April 2013 18:30, Manuel Quiñones <manuq at laptop.org> wrote:
>>> >> >>
>>> >> >> Hi devs,
>>> >> >>
>>> >> >> I've seen some movement in current master.  I suppose all this was
>>> >> >> reviewed, but what I wonder is if we are sticking to the
>>> signed-off-by
>>> >> >> / acked-by signing.
>>> >> >>
>>> >> >> The commits in question are:
>>> >> >>
>>> >> >> commit f423ec21b4bf0d953a470a383cc801b61a087e98
>>> >> >> Author: Walter Bender <walter.bender at gmail.com>
>>> >> >> Date:   Sat Mar 30 16:06:00 2013 -0400
>>> >> >>
>>> >> >>     Add comment box to expanded entry
>>> >> >>
>>> >> >> commit 541af0166030a5f3b7b52bdc23d416dbf688b5e9
>>> >> >> Author: Walter Bender <walter.bender at gmail.com>
>>> >> >> Date:   Sat Mar 30 10:45:22 2013 -0400
>>> >> >>
>>> >> >>     Add CommentView widget to expanded entry
>>> >> >>
>>> >> >> commit 248a758875e5a02a53ae57980f4fe2f3bb0bb91e
>>> >> >> Author: Walter Bender <walter.bender at gmail.com>
>>> >> >> Date:   Thu Mar 28 15:26:17 2013 -0400
>>> >> >>
>>> >> >>     Make a separate method for _write_entry so method can be reused
>>> >> >>
>>> >> >> commit 1af4b5ec7f03ed43da1616df946fe936e8577e91
>>> >> >> Author: Walter Bender <walter.bender at gmail.com>
>>> >> >> Date:   Thu Mar 28 15:25:41 2013 -0400
>>> >> >>
>>> >> >>     Pass a widget to _create_scrollable so method can be reused
>>> >> >>
>>> >> >>
>>> >> >> --
>>> >> >> .. manuq ..
>>> >> >> _______________________________________________
>>> >> >> Sugar-devel mailing list
>>> >> >> Sugar-devel at lists.sugarlabs.org
>>> >> >> http://lists.sugarlabs.org/listinfo/sugar-devel
>>> >> >
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Daniel Narvaez
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> .. manuq ..
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> > Daniel Narvaez
>>>
>>>
>>>
>>> --
>>> .. 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/20130404/010e0a34/attachment.html>