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/00669...
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 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.