On Wed, Sep 12, 2012 at 4:07 PM, Edison Su <[email protected]> wrote: > > >> -----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.
An recommendation on "How to submit patches for CloudStack" would be helpful. > >> >> Or at least, follow the policy that: >> 1. Contributor is author >> 2. Committer need to provide Signed-off. And this policy should able to be enforced by committers. --Sheng >> >> --Sheng >> >> >> >> --Sheng >> >> > >> >> > -chip
