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? Or at least, follow the policy that: 1. Contributor is author 2. Committer need to provide Signed-off. --Sheng >> >> --Sheng >> > >> > -chip
