On 02/04/16 12:53, Ryan Harkin wrote:
On 3 February 2016 at 17:38, Leif Lindholm leif.lindholm@linaro.org wrote:
On Wed, Feb 03, 2016 at 05:29:58PM +0000, Carsey, Jaben wrote:
-----Original Message----- From: Ryan Harkin [mailto:ryan.harkin@linaro.org] Sent: Wednesday, February 03, 2016 9:25 AM To: Carsey, Jaben jaben.carsey@intel.com Cc: edk2-devel@ml01.01.org; linaro-uefi@lists.linaro.org; Leif Lindholm leif.lindholm@linaro.org; Justen, Jordan L jordan.l.justen@intel.com; Ard Biesheuvel ard.biesheuvel@linaro.org 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 jaben.carsey@intel.com 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_t...
(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:edk2-devel-bounces@lists.01.org] On Behalf Of Ryan Harkin Sent: Wednesday, February 03, 2016 9:10 AM To: edk2-devel@ml01.01.org; linaro-uefi@lists.linaro.org; Leif Lindholm leif.lindholm@linaro.org Cc: Ryan Harkin ryan.harkin@linaro.org; Ard Biesheuvel ard.biesheuvel@linaro.org 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 ryan.harkin@linaro.org
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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel