Re: [PATCH] git-p4: Corrected typo.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]