On 2014-08-04 16:21:27, Carsey, Jaben wrote: > > From: Justen, Jordan L > > On 2014-08-04 13:28:12, Carsey, Jaben wrote: > > > > From: Justen, Jordan L > > > > On 2014-08-01 17:13:45, Carsey, Jaben wrote: > > > > > 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?
To know who wrote the code in the tree. > > 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... You can point out where they should look. If they find it, and fix it, then they may be able to provide better patches in the future. The contributor may prefer to develop the whole patch themselves. If you added code to their patch, then I think you'd want to add 'Contributed-under' and 'Signed-off-by'. (And maybe a short one-line note about what you added, like I mentioned above.) -Jordan > > > > > 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
