Web lists-archives.com

Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak




Lars Schneider <larsxschneider@xxxxxxxxx> writes:

>> git-p4.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/git-p4.py b/git-p4.py
>> index eab319d76e..351d1ab58e 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -582,7 +582,7 @@ def currentGitBranch():
>>         # on a detached head
>>         return None
>>     else:
>> -        return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
>> +        return read_pipe(["git", "symbolic-ref", "HEAD"]).strip()[11:]
>> 
>> def isValidGitDir(path):
>>     return git_dir(path) != None
>
> Following your explanation this patch looks good to me and this fixes the
> test failure. TBH I never thought about the difference of these commands
> before. "rev" and "ref" sound so similar although they denote completely 
> different things.

Thanks for testing.

The above was done merely to point out the problematic place and a
possible solution.  As I am not familiar with the code in git-p4.py,
I didn't even try to check if the code already has a helper function
that strips "refs/heads/" from the beginning of the string (iow, I
am not happy with the [11:]).  I didn't and don't like the fact that
this function now runs "symbolic-ref HEAD" twice but I didn't try to
see if there are more suitable and idiomatic ways to do this with a
single invocation.

Hence, I would really prefer not to commit mine myself.  I'd rather
see somebody from git-p4 circle to come up with a version that is
more in line with the way things are done in the existing code and
send a tested version for me to apply.

Thanks.