| Message ID | 1435084551-32654-1-git-send-email-roy.franz@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Jun 23, 2015 at 11:39 AM, Laszlo Ersek <lersek@redhat.com> wrote: > On 06/23/15 20:35, Roy Franz wrote: >> Add a description of how to describe the authorship of a patch that >> is submitted by someone other than the original author. >> Add mention of git format-patch --stat=120 option for generating >> more useful patch names in diffstat. > > FWIW I use: > > --stat=1000 --stat-graph-width=20 > > This makes the pathname column as wide as necessary (but not wider), > while keeping the stats column to the right reasonable. I will update that (or Michael can when he commits). I recall you posting the format-patch options earlier, but searched the list and couldn't find them, so the --stat=120 was my guess. I figured I'd update the docs so this gets captured someplace that can be found :) > >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Roy Franz <roy.franz@linaro.org> >> --- >> MdePkg/Contributions.txt | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/MdePkg/Contributions.txt b/MdePkg/Contributions.txt >> index f87cbd7..98de958 100644 >> --- a/MdePkg/Contributions.txt >> +++ b/MdePkg/Contributions.txt >> @@ -67,7 +67,16 @@ Patch content inline or attached >> * The first line of commit message is taken from the email's subject >> line following [PATCH]. The remaining portion of the commit message >> is the email's content until the '---' line. >> -* git format-patch is one way to create this format >> +* git format-patch is one way to create this format. In order to get >> + useful path names in the diffstat, the "--stat=120" option should >> + be used. >> +* If a patch is being submitted by someone other than the orginal >> + author, then the orginal author's Signed-off-by/Contributed-under lines >> + should be first, followed by the Signed-off-by/Contributed-under lines >> + of the patch submitter. Any changes made by the submitter should be >> + noted above the submitter's Signed-off-by line. If git is being used >> + to prepare the patches, the git author of the commit corresponding to >> + the patch should be owned by the original author (git commit --author). > > This matches how I think about it, yes. > > Thanks > Laszlo > >> >> === Definitions for sample patch email === >> >> > ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
On Wed, Jun 24, 2015 at 4:23 AM, Olivier Martin <Olivier.Martin@arm.com> wrote: > I think your change should be integrated to a new top file 'Contributions.txt' and all the package 'Contributions.txt' should refer to this file. > Otherwise we will start to get some inconsistencies between 'Contributions.txt'. edk2$ find . -name Contributions.txt | wc -l 38 Ouch! (At least they are all identical.) I found the MdePackage instance (google brought me there), and didn't look to see if there were more.... These should definitely be replaced with a single top-level instance. Any modules that require different content can add a specialized one, but since they are all identical, that is a current requirement. Once the content of changes is agreed upon, I'll update my patch to do this as well. Roy > > -----Original Message----- > From: Roy Franz [mailto:roy.franz@linaro.org] > Sent: 23 June 2015 19:36 > To: edk2-devel@lists.sourceforge.net; lersek@redhat.com; michael.d.kinney@intel.com > Subject: [edk2] [PATCH] MdePkg: Describe submission of a patch authored by someone else > > Add a description of how to describe the authorship of a patch that is submitted by someone other than the original author. > Add mention of git format-patch --stat=120 option for generating more useful patch names in diffstat. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Roy Franz <roy.franz@linaro.org> > --- > MdePkg/Contributions.txt | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/MdePkg/Contributions.txt b/MdePkg/Contributions.txt index f87cbd7..98de958 100644 > --- a/MdePkg/Contributions.txt > +++ b/MdePkg/Contributions.txt > @@ -67,7 +67,16 @@ Patch content inline or attached > * The first line of commit message is taken from the email's subject > line following [PATCH]. The remaining portion of the commit message > is the email's content until the '---' line. > -* git format-patch is one way to create this format > +* git format-patch is one way to create this format. In order to get > + useful path names in the diffstat, the "--stat=120" option should > + be used. > +* If a patch is being submitted by someone other than the orginal > + author, then the orginal author's Signed-off-by/Contributed-under > +lines > + should be first, followed by the Signed-off-by/Contributed-under > +lines > + of the patch submitter. Any changes made by the submitter should be > + noted above the submitter's Signed-off-by line. If git is being used > + to prepare the patches, the git author of the commit corresponding to > + the patch should be owned by the original author (git commit --author). > > === Definitions for sample patch email === > > -- > 2.1.4 > > > ------------------------------------------------------------------------------ > Monitor 25 network devices or servers for free with OpManager! > OpManager is web-based network management software that monitors > network devices and physical & virtual servers, alerts via email & sms > for fault. Monitor 25 devices for free with no restriction. Download now > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 > > > ------------------------------------------------------------------------------ > Monitor 25 network devices or servers for free with OpManager! > OpManager is web-based network management software that monitors > network devices and physical & virtual servers, alerts via email & sms > for fault. Monitor 25 devices for free with no restriction. Download now > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
On Wed, Jun 24, 2015 at 11:46 AM, Kinney, Michael D <michael.d.kinney@intel.com> wrote: > Roy, > > I do not have any issues with adding this detailed description when multiple authors are involved in a single patch. > > In general, I think it would be good to avoid having multiple Signed-off-by in a single patch. From one perspective, it could be possible for the patch to be broken up into multiple patches with a different Signed-off-by in each patch. But I can imagine cases, where that actually makes the code changes in patches difficult to review and understand. So this is really just providing details on how to merge patches from multiple authors into a single patch. Is my understating correct? > > Thanks, > > Mike > Hi Mike, The main way this comes up is when someone posts a patch in response to a patch series or a question on how to do something. What prompted my patch to Contributions.txt was I asked a question, and Laszlo very helpfully posted a patch to solve my problem. His patch belongs as part of my series (implementing a new terminal type), so I will be submitting patches authored by me, as well as the one by Laszlo. It gets very difficult to coordinate submissions if my patchset now becomes dependent on a patch that Laszlo would need to submit - interdependent patchsets developed by multiple people adds a lot of overhead, and is nice to avoid when possible. It will be a minority of patches that will have multiple signed-off-by lines, but I wanted to document the proper handling of this. I think that the practice of providing patches in response to questions or patchsetset problems is a good one that we don't want do discourage. Thanks, Roy > -----Original Message----- > From: Roy Franz [mailto:roy.franz@linaro.org] > Sent: Wednesday, June 24, 2015 11:17 AM > To: edk2-devel@lists.sourceforge.net > Cc: lersek@redhat.com; Kinney, Michael D > Subject: Re: [edk2] [PATCH] MdePkg: Describe submission of a patch authored by someone else > > On Wed, Jun 24, 2015 at 4:23 AM, Olivier Martin <Olivier.Martin@arm.com> wrote: >> I think your change should be integrated to a new top file 'Contributions.txt' and all the package 'Contributions.txt' should refer to this file. >> Otherwise we will start to get some inconsistencies between 'Contributions.txt'. > > edk2$ find . -name Contributions.txt | wc -l > 38 > > Ouch! (At least they are all identical.) > > I found the MdePackage instance (google brought me there), and didn't > look to see if there were more.... > > These should definitely be replaced with a single top-level instance. > Any modules that require different content > can add a specialized one, but since they are all identical, that is a > current requirement. > > Once the content of changes is agreed upon, I'll update my patch to do > this as well. > > Roy > > >> >> -----Original Message----- >> From: Roy Franz [mailto:roy.franz@linaro.org] >> Sent: 23 June 2015 19:36 >> To: edk2-devel@lists.sourceforge.net; lersek@redhat.com; michael.d.kinney@intel.com >> Subject: [edk2] [PATCH] MdePkg: Describe submission of a patch authored by someone else >> >> Add a description of how to describe the authorship of a patch that is submitted by someone other than the original author. >> Add mention of git format-patch --stat=120 option for generating more useful patch names in diffstat. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Roy Franz <roy.franz@linaro.org> >> --- >> MdePkg/Contributions.txt | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/MdePkg/Contributions.txt b/MdePkg/Contributions.txt index f87cbd7..98de958 100644 >> --- a/MdePkg/Contributions.txt >> +++ b/MdePkg/Contributions.txt >> @@ -67,7 +67,16 @@ Patch content inline or attached >> * The first line of commit message is taken from the email's subject >> line following [PATCH]. The remaining portion of the commit message >> is the email's content until the '---' line. >> -* git format-patch is one way to create this format >> +* git format-patch is one way to create this format. In order to get >> + useful path names in the diffstat, the "--stat=120" option should >> + be used. >> +* If a patch is being submitted by someone other than the orginal >> + author, then the orginal author's Signed-off-by/Contributed-under >> +lines >> + should be first, followed by the Signed-off-by/Contributed-under >> +lines >> + of the patch submitter. Any changes made by the submitter should be >> + noted above the submitter's Signed-off-by line. If git is being used >> + to prepare the patches, the git author of the commit corresponding to >> + the patch should be owned by the original author (git commit --author). >> >> === Definitions for sample patch email === >> >> -- >> 2.1.4 >> >> >> ------------------------------------------------------------------------------ >> Monitor 25 network devices or servers for free with OpManager! >> OpManager is web-based network management software that monitors >> network devices and physical & virtual servers, alerts via email & sms >> for fault. Monitor 25 devices for free with no restriction. Download now >> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/edk2-devel >> >> >> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. >> >> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 >> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 >> >> >> ------------------------------------------------------------------------------ >> Monitor 25 network devices or servers for free with OpManager! >> OpManager is web-based network management software that monitors >> network devices and physical & virtual servers, alerts via email & sms >> for fault. Monitor 25 devices for free with no restriction. Download now >> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
diff --git a/MdePkg/Contributions.txt b/MdePkg/Contributions.txt index f87cbd7..98de958 100644 --- a/MdePkg/Contributions.txt +++ b/MdePkg/Contributions.txt @@ -67,7 +67,16 @@ Patch content inline or attached * The first line of commit message is taken from the email's subject line following [PATCH]. The remaining portion of the commit message is the email's content until the '---' line. -* git format-patch is one way to create this format +* git format-patch is one way to create this format. In order to get + useful path names in the diffstat, the "--stat=120" option should + be used. +* If a patch is being submitted by someone other than the orginal + author, then the orginal author's Signed-off-by/Contributed-under lines + should be first, followed by the Signed-off-by/Contributed-under lines + of the patch submitter. Any changes made by the submitter should be + noted above the submitter's Signed-off-by line. If git is being used + to prepare the patches, the git author of the commit corresponding to + the patch should be owned by the original author (git commit --author). === Definitions for sample patch email ===
Add a description of how to describe the authorship of a patch that is submitted by someone other than the original author. Add mention of git format-patch --stat=120 option for generating more useful patch names in diffstat. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Roy Franz <roy.franz@linaro.org> --- MdePkg/Contributions.txt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)