> -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Laszlo Ersek > Sent: Thursday, February 04, 2016 12:23 PM > To: Ryan Harkin <[email protected]> > Cc: Ard Biesheuvel <[email protected]>; Justen, Jordan L > <[email protected]>; [email protected]; Leif Lindholm > <[email protected]>; [email protected]; Carsey, Jaben > <[email protected]> > Subject: Re: [edk2] [PATCH v2 6/6] Revert "ArmPlatformPkg: Create an ARM > Platform DSC / FDF / ArmPlatformLib template" > Importance: High > > 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. >
You're right that these don't fix the issue that I brought up. My impression is that there is no direct solution so far. But these are all excellent ideas and methods to improve the usability of the repo. I think that as much of Laszlo's idea #1, #2, #3, and #4 above can be used will be really helpful. -Jaben > 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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
