Hi Thomas, I've just sent out the patches to the mailing list. I'm looking forward to receive some feedback from you :) I also have some branch detection related patch prepared. Do you use this feature often? Thanks, Vitor On Sat, Jan 29, 2011 at 2:41 AM, Vitor Antunes <vitor.hda@xxxxxxxxx> wrote: > Hi Thomas, > > First of all I'd like to thank you on your feedback. It's my first try > on creating submitting a patch, so having someone's guidance helps a > lot :) > > I'll rebase my patches against the head of the tree and squash the fix > to avoid multiple commits. While I do that I'll also review my commit > message and sign-off the patches according to what you said. Hopefully > I will be able to do this during this weekend. > > From git-diff-tree man page: > > """ > -M[<n>] > Detect renames. If n is specified, it is a is a threshold on the > similarity index (i.e. amount of addition/deletions compared to the > file’s > size). For example, -M90% means git should consider a delete/add > pair to be a rename if more than 90% of the file hasn’t changed. > """ > > But from my latest tests I think that this option is ignored in > diff-tree (I think it's only used in git log). With this in mind I'll > need to add some code to implement the check of the score value of > diff-tree output string. Again from its man page: > > """ > Status letters C and R are always followed by a score (denoting the > percentage of similarity between the source and target of the move or > copy), and are the only ones to be so. > """ > > Thanks, > Vitor > > On Fri, Jan 28, 2011 at 3:19 PM, Thomas Berg <merlin66b@xxxxxxxxx> wrote: >> Hi, >> >> On Fri, Jan 28, 2011 at 12:35 AM, Vitor Antunes <vitor.hda@xxxxxxxxx> wrote: >>> Hi everyone, >>> >>> Could anyone comment the 3 patches I sent (being this the last one)? >>> >> [...] >>> On Thu, Nov 25, 2010 at 1:26 AM, Vitor Antunes <vitor.hda@xxxxxxxxx> wrote: >>>> --- >>>> contrib/fast-import/git-p4 | 2 +- >>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4 >>>> index 0ea3a44..a466847 100755 >>>> --- a/contrib/fast-import/git-p4 >>>> +++ b/contrib/fast-import/git-p4 >>>> @@ -618,7 +618,7 @@ class P4Submit(Command): >>>> if len(detectRenames) > 0: >>>> diffOpts = "-M%s" % detectRenames >>>> else: >>>> - diffOpts = ("", "-M")[self.detectRenames] >>>> + diffOpts = ("", "-M")[self.detectRename] >>>> >> >> This appears to me to be a bugfix for one of the other patches you >> sent, is that right? >> >> If so, maybe you could squash it with the previous patch and re-send >> it all to the list? >> >> My other comments for now are: >> - you have forgotten to sign off on the patches >> - commit messages are normally in imperative rather than past tense >> (see Documentation/SubmittingPatches in git) >> >> - In your first patch you wrote: >>> The detectRenames option should be set to the desired threshold value. >> I'm not sure what threshold value you refer to here, and what values >> you can set it to. Am I missing something? >> (I'm not very familiar with git rename detection options) >> >> I'm a git-p4 user, so I can test your changes and look a bit more at >> your code. Someone verifying it could help getting the patches >> applied. >> >> Thanks for improving git-p4! >> >> Cheers, >> Thomas Berg >> > > > > -- > Vitor Antunes > -- Vitor Antunes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html