On 2014-08-04 13:28:12, Carsey, Jaben wrote: > > -----Original Message----- > > From: Justen, Jordan L > > Sent: Friday, August 01, 2014 6:03 PM > > To: edk2-devel@lists.sourceforge.net; Carsey, Jaben; edk2- > > de...@lists.sourceforge.net > > Subject: Re: [edk2] [RFC 1/2] EDK II Contributions.txt: Update patch format > > information > > Importance: High > > > > On 2014-08-01 17:13:45, Carsey, Jaben wrote: > > > Jordan, > > > > > > Will this affect MdePkg only or all packages? (I notice that there is > > > a copy of this file in many if not all packages) Is the plan to copy > > > this change to other copies? Are all copies the same? > > > > Sorry. Yes, I meant to mention that. I didn't want to make a huge patch > > will all > > duplicate information. > > > > Yes, they should all be identical copies. > > > > > On to the main part: > > > > > > I do not agree with this change. > > > > > > I prefer the attached patches and have to request them when I get > > > inline patches. As an SVN user, I find git-based inline patches very > > > challenging to use. > > > > This seems like something we should look into. Maybe we can figure out if > > this can be addressed with some additional tools. (And hopefully > > documented for others.) > > Agreed. This would mitigate lots of my issues. > > > > > Well, I guess there is always git-svn. :) > > > > > I do not mind if other packages both formats. > > > > I guess today we basically say we will accept either format today. And > > sometimes the package owner asked if the contributor can do something > > different. > > Agreed. > > > > > > I find that replying with another patch is often much more useful than > > > I think replying inline to a patch would be (having not tried it per > > > above). > > > > This seems potentially problematic from a copyright standpoint. Who's code > > is it then? I usually prefer to just give some non-code feedback. > > If I apply their patch, and then make changes and update the > copyright to any file that I modify, then the copyrights are fine. > There is no need to worry about copyrights.
I'm not meaning the copyright notices in the file. I'm referring to being able to scan the repo history, look at the author, and figure out what changed lines were written in that commit. That would be the person with the Signed-off-by, or the separate patch author for git. Anyway, you can see how that gets a bit confusing if multiple authors edit a single patch. (I'm not saying it doesn't happen, just that it might be nice to try to avoid it.) > > Of course, sometimes it is really hard to not get into the code. In those > > cases > > I add this to the commit message: > > [jordan.l.jus...@intel.com: did some stuff] > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-of-by: Jordan Justen <jordan.l.jus...@intel.com> > > > > But, I do like to make that the exception for patch feedback rather than the > > rule. > > > > Anyway, I find it really nice (for code review) to just hit reply, and then > > add > > comments right where the review is needed for the patch. (Rather than > > copying, pasting, quoting, etc.) > > Normally I find that what I am commenting on is more like "this > piece of code here also must be updated"; so it's more of an > addendum to the patch, rather than a comment on it. My returned > patch is just their patch with another change also thrown in. Can't this often be written in non-code as: * You should return FOO in bar scenario, or * Does this handle the BAZ case? > > > Can you clarify what this means: > > > +* This format matches git's format-patch command. > > > > To let git users know they can use 'git format-patch'. > > > > > +* Patch files can be attached, but inline is preferred. > > > > > > Do you mean the patch email format or the patch itself? > > > > For git, these are one in the same. 'git format-patch' makes a patch/email > > message. Then you use 'git send-email' to send the patch/email to the list. > > Then anyone on the list can download the email, and run 'git am' on the > > email > > message to incorporate the patch (commit messsage, authorship and all) > > right into their tree. > > > > This is why no one's commit messages or patches get screwed up with git. > > The tools take care of the whole process. > > > > But, for the 'attach' use case, it would be referring to the patch. (Which, > > I > > guess could be what git format-patch generated too.) > > As long as you have a documented process for SVN, I don't mind. I > don’t want to remove support for something that is still used by > many users. Under 'Notes' my patch also added: "Patch files can be attached, but inline is preferred." So, it doesn't appear to remove support for anything. I still hope we can migrate this direction so we can easily provide feedback directly to the patch email. My experience is that this is the easiest way to provide email based review feedback. Hmm, totally unrelated thought... I wonder if we might consider adding: If you push your patches to an external git repo you can provide a link to the branch in your patch series cover letter, or just below the '---' line that marks the end of the commit message. -Jordan > > > -----Original Message----- > > > From: Jordan Justen [mailto:jordan.l.jus...@intel.com] > > > Sent: Friday, August 01, 2014 3:52 PM > > > To: edk2-devel@lists.sourceforge.net > > > Subject: [edk2] [RFC 1/2] EDK II Contributions.txt: Update patch > > > format information > > > > > > Update to show what the patch looks like in email form. > > > > > > Note a preference for inline patches. > > > > > > NOTE: This does not modify the wording of the "TianoCore Contribution > > > Agreement 1.0" section > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> > > > --- > > > MdePkg/Contributions.txt | 40 > > > +++++++++++++++++++++++++++------------- > > > 1 file changed, 27 insertions(+), 13 deletions(-) > > > > > > diff --git a/MdePkg/Contributions.txt b/MdePkg/Contributions.txt index > > > 667ca10..dd1c6c3 100644 > > > --- a/MdePkg/Contributions.txt > > > +++ b/MdePkg/Contributions.txt > > > @@ -20,9 +20,9 @@ To make a contribution to a TianoCore project, follow > > these steps. > > > not documented, then submit the code on development email list > > > for the project. > > > > > > -======================================= > > > -= Change Description / Commit Message = > > > -======================================= > > > +===================================================== > > > += Change Description / Commit Message / Patch Email = > > > +===================================================== > > > > > > Your change description should use the standard format for a commit > > message, and must include your "Signed-off-by" signature @@ -30,7 +30,30 > > @@ and the "Contributed-under" message. > > > > > > == Sample Change Description / Commit Message = > > > > > > -=== Definitions for sample change description === > > > +=== Start of sample patch email message === > > > + > > > +From: Contributor Name <contributor@email.server> > > > +Subject: CodeModule: Brief-single-line-summary > > > + > > > +Full-commit-message > > > + > > > +Contributed-under: TianoCore Contribution Agreement 1.0 > > > +Signed-off-by: Contributor Name <contributor@email.server> > > > +--- > > > +PATCH INCLUDED INLINE > > > + > > > +=== End of sample patch email message === > > > + > > > +=== Notes for sample patch email === > > > + > > > +* This format matches git's format-patch command. > > > +* Patch files can be attached, but inline is preferred. > > > + - Replying to comment on a patch included inline is much easier. > > > +* The first line of commit message is taken from the email's subject > > > + line. The remaining portion of the commit message is the email's > > > + content until the '---' line. > > > + > > > +=== Definitions for sample patch email === > > > > > > * "CodeModule" is a short idenfier for the affected code. For > > > example MdePkg, or MdeModulePkg UsbBusDxe. > > > @@ -44,15 +67,6 @@ and the "Contributed-under" message. > > > * "Signed-off-by" is the contributor's signature identifying them > > > by their real/legal name and their email address. > > > > > > -=== Start of sample change description / commit message === > > > -CodeModule: Brief-single-line-summary > > > - > > > -Full-commit-message > > > - > > > -Contributed-under: TianoCore Contribution Agreement 1.0 > > > -Signed-off-by: Contributor Name <contributor@email.server> -=== End > > > of sample change description / commit message === > > > - > > > ======================================== > > > = TianoCore Contribution Agreement 1.0 = > > > ======================================== > > > -- > > > 2.0.1 > > > > > > > > > ---------------------------------------------------------------------- > > > -------- Want fast and easy access to all the code in your enterprise? > > > Index and search up to 200,000 lines of code with a free copy of Black > > > Duck Code Sight - the same software that powers the world's largest > > > code search on Ohloh, the Black Duck Open Hub! Try it now. > > > http://p.sf.net/sfu/bds > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > > > ---------------------------------------------------------------------- > > > -------- Want fast and easy access to all the code in your enterprise? > > > Index and search up to 200,000 lines of code with a free copy of Black > > > Duck Code Sight - the same software that powers the world's largest > > > code search on Ohloh, the Black Duck Open Hub! Try it now. > > > http://p.sf.net/sfu/bds > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-devel
signature.asc
Description: signature
------------------------------------------------------------------------------ Infragistics Professional Build stunning WinForms apps today! Reboot your WinForms applications with our WinForms controls. Build a bridge from your legacy apps to the future. http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
