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