> -----Original Message----- > From: Sheng Yang [mailto:[email protected]] > Sent: Wednesday, September 12, 2012 4:04 PM > To: [email protected] > Subject: Re: [DISCUSS]Patch format/applying policy for contributors and > committers > > On Wed, Sep 12, 2012 at 3:36 PM, Edison Su <[email protected]> wrote: > > > > > >> -----Original Message----- > >> From: Sheng Yang [mailto:[email protected]] > >> Sent: Wednesday, September 12, 2012 2:36 PM > >> To: [email protected] > >> Subject: Re: [DISCUSS]Patch format/applying policy for contributors > and > >> committers > >> > >> On Wed, Sep 12, 2012 at 1:42 PM, Chip Childers > >> <[email protected]> wrote: > >> > On Wed, Sep 12, 2012 at 4:35 PM, Sheng Yang <[email protected]> > wrote: > >> >> Hi, > >> >> > >> >> I think we would need some way to make the commit more clear. > >> >> > >> >> Currently committers are committed the patch in any format he/she > >> >> want. E.g. using reviews.apache.org 's ticket number as subject, > add > >> >> some "Contributed-by:" or "Sent-by:" or "Wrote-by:" in the > context > >> to > >> >> identify the contributor(but committer themselves remained > "Author"), > >> >> which is inconsistent across the committers, and hard for > statistics. > >> >> > >> >> Also many commits lacks of proper explanation and subjects to be > >> >> easily identified. > >> >> > >> >> So the best way to me, is let contributor listed as Author in the > >> >> commit, and committer can "Signed-off" on it. > >> >> > >> >> I suggest we can follow the Linux Kernel community case on this > >> issue. > >> >> > >> >> 1. the contributor should provide git-format-patch generated > patch, > >> >> which is able to indicate who is the author and what's to fix. > >> >> 2. Committer should use git-am to apply the patch, add Signed- > off-by: > >> >> xxxx([email protected]) at end of it, to indicate who agreed and > >> >> committed this patch(in fact there are more potential rules here, > >> but > >> >> let's stick with the simplest one first). > >> >> > >> >> Then author need to write the subject and more detail explaining > of > >> >> the patch if necessary, and it would be easier for others to > track > >> >> which patch is from who on what issue, and what's the solution. > >> >> > >> >> Another advantage/disadvantage of git-am is, it's very strict. > You > >> >> have provided up-to-date patch, otherwise it would fail. No fuzz > is > >> >> allowed. > >> >> > >> >> One problem currently is reviews.apache.org doesn't accept the > git > >> >> formatted patch, it would only accept diff. So if we want to get > >> patch > >> >> from reviews.apache.org, it would result in committer have to > find > >> >> subject and detail explanation for the patch, which may varies > from > >> >> author's, and also increase committer's workload. > >> >> > >> >> I think probably we can let contributor send out two mail for > each > >> >> patch? One attached latest patch, another one which is sent by > >> >> reviews.apache.org, provided a convenient way for reviewing? The > >> >> alternative way of doing is let contributor send out > >> >> git-format-patch-ed patch(which would be based on the latest code) > >> >> after committer reviewed patch and think it's ready to be applied. > >> >> > >> >> --Sheng > >> >> > >> > > >> > Hey Sheng, > >> > > >> > Given the current limits of reviews.a.o, I've recently turned to > >> using > >> > this process: > >> > > >> > git apply foo.patch > >> > git commit -a -s --author='Person Name <[email protected]>' > >> > > >> > Then, I've been putting in the reviewboard summary and description > >> > into the commit comment (sometimes edited to help with clarity), > >> which > >> > should be easily available since the patch was just downloaded > from > >> > reviewboard anyway. > >> > > >> > This gives us the correct author, correct committer and a decent > >> commit message. > >> > >> Hi Chip, > >> > >> Yes, this works, though still committer need to do some tweak on the > >> patches, and more you do, the more chance you got to mess up > >> something. To me, what's the advantage of git-am is, it would > >> guarantee what's original author wanted information is there, as > well > >> as code is intact. > >> > >> Currently I think your approach is good, but we need to make it as a > >> policy and require every committer to do the same as well. > >> > > >> > Personally, I'd like to avoid double emails. We also know that > the > >> > dev list both truncates (and sometimes mangles) patches sent in > the > >> > text of emails, as well as stripping attachments out. Would the > >> > method I'm describing above work for you? > >> > >> I think it's fine for now, but when volume of patches and workload > >> increase, I suppose we still need to figure out some way more > >> directly. > > > > http://wiki.cloudstack.org/display/gen/Review%20Board > > http://markmail.org/message/zuppmf7un7pt6q65 > > Rohit, had a tool to send original patch to review board, committer > can easily download the patch from a link in the review board, the "git > apply". > > Yes, this is nice! > > Could we make it mandatory?
You can't make it mandatory, as people always can upload patch from review board UI, instead of command line. > > Or at least, follow the policy that: > 1. Contributor is author > 2. Committer need to provide Signed-off. > > --Sheng > >> > >> --Sheng > >> > > >> > -chip
