> -----Original Message----- > From: Justen, Jordan L > Sent: Monday, August 04, 2014 4:16 PM > To: Carsey, Jaben; edk2-devel@lists.sourceforge.net > Subject: Re: [edk2] [RFC 1/2] EDK II Contributions.txt: Update patch format > information > Importance: High > > 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.
I am not sure why we are worried about this. What is the motivation for this feature for us? > > 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? Could, but as the package owner often it's faster to write a few lines of code than to send someone on a search of the code for some feature... > > > > > 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 ------------------------------------------------------------------------------ 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
