From: Vitor Antunes <vitor.hda@gmail.com>
Date: Sun, 30 Jan 2011 23:21:50 GMT
Subject: Re: [PATCH] git-p4: Corrected typo.
Message-ID: <AANLkTi=PPN69uuJmUBDHKtmn59DzUbdk=Qu4Ug-kok89@mail.gmail.com>
In-Reply-To: <AANLkTimLQxiB=dnd6=iB5uEpzOSfqks=FDJ=xORehuw8@mail.gmail.com>

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@gmail.com> 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@gmail.com> wrote:
>> Hi,
>>
>> On Fri, Jan 28, 2011 at 12:35 AM, Vitor Antunes <vitor.hda@gmail.com> 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@gmail.com> 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