[Webkit-unassigned] [Bug 171450] Add a new function for getting the Git hash for a pure git directory.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 28 15:19:21 PDT 2017


https://bugs.webkit.org/show_bug.cgi?id=171450

--- Comment #4 from Jason Marcell <jmarcell at apple.com> ---
Comment on attachment 308595
  --> https://bugs.webkit.org/attachment.cgi?id=308595
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308595&action=review

>> Tools/Scripts/VCSUtils.pm:451
>> +    my $revision;
> 
> I know that the variable name $revision is consistent with the other functions of this ilk, but you might consider calling it $commit or $hash to match the name of your function.

Yep. I guess I'll go with $commit unless anyone objects.

>>> Tools/Scripts/VCSUtils.pm:454
>>> +        warn "$directory is a pure git repo.";
>> 
>> Is this debugging code?
> 
> I assume we also don't mind that git-svn repos will execute this code path.

This is debugging code (which I will remove before landing). This function will work fine for a git-svn repo, however, we wouldn't/shouldn't use it because there is an already-existing function called `svnRevisionForDirectory` (which, not coincidentally, this function was heavily based on).

>> Tools/Scripts/VCSUtils.pm:456
>> +        $command = "LC_ALL=C $command" if !isWindows();
> 
> Curious what is this for?

I'm not sure. It's some copy pasta from `svnRevisionForDirectory`. You're right to question it. We should figure out if we even need it here before landing.

>> Tools/Scripts/VCSUtils.pm:458
>> +        chomp($revision)
> 
> Missing semicolon.

Will fix.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170428/73856fb9/attachment-0001.html>


More information about the webkit-unassigned mailing list