On 11-Sep-2008, Stefano Zacchiroli wrote:
> Thanks a lot: I've investigated with the other maintainers and no 
> voices against your request were raised, we will be happy to 
> integrate your patch (and thanks for the deprecation module, which 
> will probably be useful also for other stuff in the future).

Great news, thank you.

> Still, your patch at the moment is a bit messy and suboptimal to review.
> Take for example the patch about the example
> examples/debtags/tagsByRelevance; your patch first remove all the code
> and then read it, probably using the right calls.

As far as I can tell, that's an artefact of the way Git handles file 
metadata: it can't tell the difference between "file X was renamed to 
file Y and had some changes" and "file X lost all 100 lines and file Y 
gained all 100 very similar lines".

I may be wrong on this (I'm not an expert at using Git) but Git, as 
far as I can tell, is just representing the change as best it can.

You'll notice that Git has produced more readable output for the other 
method-renaming patch (the one that changes many more files).

> Can you please:
> - provide 2 patches instead of 3: one for the deprecation module, the
>   other (instead of 2) for all the method renamings.

In fact the patch to 'examples/debtags/tagsbyrelevance' is only 
separate because I missed it on the first pass. Combining the two 
patches won't help the readability, as it will still contain a whole 
lot of removed lines in one file and a whole lot of added lines in the 
other.

I was a little dismayed to see that the project decided to move away 
from Bazaar, which handles this case more elegantly :-/

> - avoid stuff like the example above and keep your patches minimal.
> 
> Once that is ready, I'll be more than happy to review, apply, and
> upload.

I hope you can reconsider based on the above; I don't see how to 
improve my patches while still using Git.

-- 
 \                              “Holy rising hemlines, Batman!” —Robin |
  `\                                                                   |
_o__)                                                                  |
Ben Finney <[EMAIL PROTECTED]>

Attachment: signature.asc
Description: Digital signature

Reply via email to