Code review required for commits - formal Team vote.
Michael Adam
obnox at samba.org
Tue Oct 23 06:10:25 MDT 2012
On 2012-10-23 at 08:03 -0400, simo wrote:
> On Tue, 2012-10-23 at 08:49 +0200, Michael Adam wrote:
> >
> > tracking tool - patchwork:
> >
> > Simo has set up patchwork for the samba code.
> > See the corresponding subthread for details.
> >
> > It looks rather promising. The only thing I am not quite
> > certain about is the inability to track whole patchsets
> > handed in by mail.
>
> This came out funny, I think you meant:
> patchwork can't handle multiple patches in a single mail message.
Er, yes, that is what I tried to express. :-)
Interestingly, patchwork seems to have the concept of patch
bundles internally.
> > git send-email:
> >
> > There have been various hints and discussions about
> > using git send-email.
> > It would be great to collect these on a wiki page.
>
> can you create a page and send the link to the Mailing List ?
> I will put my advice in there.
Yeah, I can do that.
I personally prefer to use "git format-patch" and attach the output
to mails. If working with patchsets, I use "git format-patch
--stdout" to create a single mbox-style file instead of having
to deal with lots of attachment files or even (worse) lots of
mails for one logical unit. But maybe it is only a matter of
getting used to ot
> > commit messsage tags:
> >
> > ...
>
> Note that with 2 sign-offs this becomes effectively redundant, so I
> would phase it out to keep things simple.
Right. But that redundancy would clearly separate it from our
earlier (mis-)use where we frequently added sign-off tags
to indicate review.
> > - A reviewer should add a "Reviewed-by: " tag.
> >
> > Here is a reference for the linux kernel document describing
> > the various tags used by linux:
> > http://www.kernel.org/doc/Documentation/SubmittingPatches
> >
> > There is also the "Acked-by: " patch, but I suggest that we
> > don't use it for now to keep things simple, since it does not
> > make a lot of sense, unless we have established maintainers
> > all over the code base and those need to Ack patches for their
> > areas.
> >
> > I also talked with some git folks, whether native support for
> > adding "reviewed-by" like for "signed-off-by" coule be added
> > to git, but I think chances are bad, because they say the don't
> > want a reviewer to change the commit message and hence the
> > commit object hash... They suggest to instead use merges and
> > use the merge commit for indicating reviewed state. I
> > personally don't like that very much, and think we should stay
> > with the message-changing pick/rebase model for now.
>
> +1
Thanks - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20121023/e5bfeb0d/attachment.pgp>
More information about the samba-technical
mailing list