On 02/04/16 12:53, Ryan Harkin wrote: > On 3 February 2016 at 17:38, Leif Lindholm <[email protected]> wrote: >> On Wed, Feb 03, 2016 at 05:29:58PM +0000, Carsey, Jaben wrote: >>>> -----Original Message----- >>>> From: Ryan Harkin [mailto:[email protected]] >>>> Sent: Wednesday, February 03, 2016 9:25 AM >>>> To: Carsey, Jaben <[email protected]> >>>> Cc: [email protected]; [email protected]; Leif Lindholm >>>> <[email protected]>; Justen, Jordan L <[email protected]>; >>>> Ard >>>> Biesheuvel <[email protected]> >>>> Subject: Re: [edk2] [PATCH v2 6/6] Revert "ArmPlatformPkg: Create an ARM >>>> Platform DSC / FDF / ArmPlatformLib template" >>>> Importance: High >>>> >>>> On 3 February 2016 at 17:20, Carsey, Jaben <[email protected]> >>>> wrote: >>>>> Change looks good. >>>>> >>>>> Question - Is there a way to optimize the patch/email for this type of >>>>> change >>>> so that we do not see the "diff" for a file that is deleted? It seems >>>> redundant >>>> to show a whole file of "-" lines... >>>>> >>>> >>>> That's a good question. I remember Ard discussing this previously. >>>> I'll see if I can find the discussion... >>> >>> Thanks! That would be good. >>> > > I can't find Ard's original message. But I've worked out that if I do > > $ git format-patch -D -1 <sha> > > It will not show all the deleted lines. I could then "git send-email" > the patch file, rather than the using the SHA. > > However, the patch will not be able to be applied because it's missing > the delete of the contents. So it's only useful for review, > unfortunately. I vaguely remember Ard mentioning that in his original > email - that I can't find!
I don't think that -D is a good idea. The patch should be applicable from the mailing list for testing & local review. I do use a number of tweaks that procude more readable patches: (1) I enable rename and copy detection. See "diff.renames" in git-config(1). (2) When appropriate, I even force copy detection for patches where the original files (being copied from) are not touched in the same patch. See "--find-copies-harder" in git-diff(1). (3) Earlier we discussed how section headers of dec, dsc, inf etc. files can be captured in diff hunk header. Most recently: please see the end of the message at http://thread.gmane.org/gmane.comp.bios.edk2.devel/7156/focus=7183 (4) I use the "patience" diff algorithm. See "diff.algorithm" in git-config(1), and git-diff(1). A good explanation can be found in the QEMU wiki, under Make code motion patches easy to review http://wiki.qemu.org/Contribute/SubmitAPatch#Make_code_motion_patches_easy_to_review (5) I order the files in my diffs so that the descriptive files go first (dec, dsc, fdf, inf), the C language header files, and HII forms, second, and the C source code last. In my experience, seeing the header changes before the C code changes makes a huge difference. For this, create a text file like: ------ *.dec *.dsc.inc *.dsc *.fdf *.inf *.h *.vfr *.c ------ and pass the pathname of this text file to git-format-patch with the "-O" option (no space between option and option argument). (6) Occasionally I increase the context from 3 lines to 5 or so (with "-U5"). None of these solve the exact request formulated by Jaben, but they all keep the patch applicable from the mailing list. Thanks Laszlo > > >>> The worst example is the patch with a delete and then some code change >>> (probably should be a series). >> >> You can tell this from the start of the diff for each file: >> >>> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc >>> +++ /dev/null , >> >> And also from the diffstat (if all '-' signs next to a file, nothing >> was added, if anything was added, you'll see '+' also). >> >>>>>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc | 382 >>>>>> ------------------ >>>>>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.fdf | 263 ------------ >>>>>> ArmPlatformPkg/ArmPlatformPkg.dsc | 445 >>>>>> --------------------- >>>>>> ArmPlatformPkg/ArmPlatformPkg.fdf | 320 --------------- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNull.c | 162 -------- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNull.inf | 51 --- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNullMem.c | 34 -- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNullSec.inf | 47 --- >> >> And, yes, it should :) >> >> / >> Leif >> >>>>>> -----Original Message----- >>>>>> From: edk2-devel [mailto:[email protected]] On Behalf Of >>>>>> Ryan Harkin >>>>>> Sent: Wednesday, February 03, 2016 9:10 AM >>>>>> To: [email protected]; [email protected]; Leif Lindholm >>>>>> <[email protected]> >>>>>> Cc: Ryan Harkin <[email protected]>; Ard Biesheuvel >>>>>> <[email protected]> >>>>>> Subject: [edk2] [PATCH v2 6/6] Revert "ArmPlatformPkg: Create an ARM >>>>>> Platform DSC / FDF / ArmPlatformLib template" >>>>>> Importance: High >>>>>> >>>>>> This reverts commit 12fcdcb83d8e91cb730db9252189269467b9ed73 >>>>>> >>>>>> The files added by the commit have never been used. In the intervening >>>>>> years since they've been added, ~30 patches have been submitted to >>>>>> update them. >>>>>> >>>>>> Remove them as part of a general cleanup of ArmPlatformPkg. >>>>>> >>>>>> Signed-off-by: Ryan Harkin <[email protected]> >>>>>> --- >>>>>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc | 382 >>>>>> ------------------ >>>>>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.fdf | 263 ------------ >>>>>> ArmPlatformPkg/ArmPlatformPkg.dsc | 445 >>>>>> --------------------- >>>>>> ArmPlatformPkg/ArmPlatformPkg.fdf | 320 --------------- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNull.c | 162 -------- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNull.inf | 51 --- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNullMem.c | 34 -- >>>>>> .../ArmPlatformLibNull/ArmPlatformLibNullSec.inf | 47 --- >>>>>> 8 files changed, 1704 deletions(-) >>>>>> > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
