Re: [PATCH v2 4/7] Rename monitor_x86.c to monitor.c and monitor_arch.h to monitor.h

[Corneliu ZUZU <czuzu@bitdefender.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2460 bytes --]

On 2/10/2016 6:44 PM, Tamas K Lengyel wrote:
>
>
> On Wed, Feb 10, 2016 at 8:54 AM, Corneliu ZUZU <czuzu@bitdefender.com 
> <mailto:czuzu@bitdefender.com>> wrote:
>
>     Rename:
>         - arch/x86/monitor_x86.c -> arch/x86/monitor.c
>         - asm-{x86,arm}/monitor_arch.h -> asm-{x86,arm}/monitor.h
>
>     (previous commit explains why these renames were necessary)
>
>
> Referencing a "previous commit" like this is not acceptable as you 
> don't know how these patches will get applied and there might be other 
> patches that get committed in-between yours. In each patch explain 
> clearly why the patch is needed. I still find it odd that you need to 
> rename x86/monitor.c -> x86/monitor_x86.c -> x86/monitor.c in the same 
> series.
>
>
>     Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com
>     <mailto:czuzu@bitdefender.com>>
>     ---
>

As I stated earlier, those commits (4 & 6) are the result of a git diff
limitation (also see my responses to these patches in v1).
The following simple experiment illustrates my point precisely:
1) create a local git repo, add file ./a.txt there with many lines (e.g. 
100) and make that commit 1
2) now create a dir ./d, move a.txt -> ./d/a.txt and add another ./a.txt 
w/ few lines, e.g. 2, make that commit 2

Git diff (even w/ the -M, -C options) of commit 2 would then report that:

     # ./a.txt has been modified => diff w/ 100 deletions & 2 additions
     # ./d/a.txt has been added => diff w/ 100 additions
     => 202 additions & deletions total, *even though* similarity 
between ./a.txt in commit 1 and
         ./d/a.txt in commit 2 is 100% ! (also even if you use -M, -C 
diff options)

*instead of* reporting

     # ./a.txt moved to ./d/a.txt, similarity index 100% => 0 deletions 
& 0 additions
     # added ./a.txt => 2 additions
     => 2 additions total

You could see why this limitation would have made the diffs somewhat bulky,
especially when those files also get to be modified in the process.
This is also the reason why accepting patch 3 without 4 or 5 without 6 
wouldn't
make sense, 3+4 should be treated as a single patch, 5+6 similarly.

As a proposed resolution: since I took these measures *only to ease code 
review*,
patches 4 & 6 can be safely squashed onto their previous commits (3 & 5) 
when and if acking them.
Or I could just worry less about this next time and simply squash them 
myself before sending
them for review, less to do on my part :).

Corneliu.

[-- Attachment #1.2: Type: text/html, Size: 4251 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel