[SSSD] Using the Reviewed-by git tag

Lukas Slebodnik lslebodn at redhat.com
Mon Feb 10 12:13:32 UTC 2014


On (10/02/14 11:03), Jakub Hrozek wrote:
>On Mon, Feb 10, 2014 at 09:26:43AM +0100, Lukas Slebodnik wrote:
>> On (09/02/14 17:00), Jakub Hrozek wrote:
>> >Hi,
>> >
>> >as we discussed with the other developers earlier this week, we should
>> >start using Reviewed-by tag. As a benefit, we would easily see which
>> >developer understands the code apart from the author without having to
>> >resort to searching the mailing list.
>> >
>> >I tried using the tag when pushing Pavel's recent patch reviewed by
>> >Stephen, so you can see the result with:
>> >git log -1 fd520622529e26682eb8fa4c5355db18399c3121
>> >
>> >For the workflow, I followed the Samba guidelines pretty much
>> >completely:
>> >https://wiki.samba.org/index.php/CodeReview
>> >
>> >Would this workflow work for everybody?
>> >
>> >Also, do we gain anything by using Signed-off-by as well? I think we
>> >wouldn't as the vast majority of patches are written by a single
>> >contributor, so adding Signed-off-by would be mostly busywork, but I'd
>> >like to hear other opinions.
>> >
>> >When we agree on the workflow, we should add it to:
>> >https://fedorahosted.org/sssd/wiki/DevelTutorials
>> >
>> 
>> I have another tip.
>> 
>> I will be nice to call git cherry-pick with argument "-x" if patch is
>> backported to older branches. It will append hash of original commit.
>> 
>>     (cherry picked from commit 56feae39a4d3c356c13d6826f34f83e0471f6e07)
>
>Does the 389 team only use this tag when there are no conflicts between
>branches or always? Some patches can significantly diverge between
>branches..
>
>> 
>> Here is a link to patch from 389-ds-base
>> https://lists.fedoraproject.org/pipermail/389-commits/2014-February/006697.html
>> 
>> BTW: They also use "Reviewed by"
>> +1
>
>I see they also use many more tags, but I don't want to impose more
>process to be honest.
>
>Also Pavel R. had a good question -- who actually adds the tag?
I am not sure. I noticed this today when I was processing unreaded messages in
my mailboxes. We should ask 389 developers (freenode/#389)

>I think
>it should be the job of whoever is pushing the patch. I think with the
>current patch volume, this wouldn't be too much of a burden. When/if we
>switch to a code review tool that is able to merge patches, the tool could
>add all the tags based on the review itself.

LS



More information about the sssd-devel mailing list