svndiff for the windows

svndiff for the windows

R=epoger@google.com

Committed: https://code.google.com/p/skia/source/detail?r=11995

[bsalomon, 2013-10-28 21:07:21]

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py
File tools/svndiff.py (right):

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode91
tools/svndiff.py:91: extension = ""
is it really dumb to init to an empty string? I don't know python.

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode210
tools/svndiff.py:210: args = ['git', 'show', 'HEAD:./' + file_within_repo]
we're passing a path from a path that was extracted from git output to another
git command. git wants to see / not \.

[epoger, 2013-10-28 21:14:40]

LGTM with our without these nits picked

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py
File tools/svndiff.py (right):

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode91
tools/svndiff.py:91: extension = ""
On 2013/10/28 21:07:22, bsalomon wrote:
> is it really dumb to init to an empty string? I don't know python.

Probably a "real" Python programmer would scoff at it, but I like it.

Note, however, that our style guide calls for single-quoted strings in Python
code.

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode95
tools/svndiff.py:95: possible_paths = [os.path.join(trunk_path, 'out',
'Release', 'skdiff' + extension),
wrap at 80 chars please (I love enforcing rules that even I don't like)

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode210
tools/svndiff.py:210: args = ['git', 'show', 'HEAD:./' + file_within_repo]
On 2013/10/28 21:07:22, bsalomon wrote:
> we're passing a path from a path that was extracted from git output to another
> git command. git wants to see / not \.

Makes sense.  Please add your explanation to the comment block, though, so
future travelers can see it.

P.S. You also have the option of calling posixpath.join() to do this.  Either
one is fine with me.

[bsalomon, 2013-10-29 13:55:47]

Committed patchset #2 manually as r11995 (presubmit successful).

[bsalomon, 2013-10-29 14:23:42]

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py
File tools/svndiff.py (right):

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode91
tools/svndiff.py:91: extension = ""
On 2013/10/28 21:14:41, epoger wrote:
> On 2013/10/28 21:07:22, bsalomon wrote:
> > is it really dumb to init to an empty string? I don't know python.
> 
> Probably a "real" Python programmer would scoff at it, but I like it.
> 
> Note, however, that our style guide calls for single-quoted strings in Python
> code.

Done.

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode95
tools/svndiff.py:95: possible_paths = [os.path.join(trunk_path, 'out',
'Release', 'skdiff' + extension),
On 2013/10/28 21:14:41, epoger wrote:
> wrap at 80 chars please (I love enforcing rules that even I don't like)

Done.

https://codereview.chromium.org/49323006/diff/1/tools/svndiff.py#newcode210
tools/svndiff.py:210: args = ['git', 'show', 'HEAD:./' + file_within_repo]
On 2013/10/28 21:14:41, epoger wrote:
> On 2013/10/28 21:07:22, bsalomon wrote:
> > we're passing a path from a path that was extracted from git output to
another
> > git command. git wants to see / not \.
> 
> Makes sense.  Please add your explanation to the comment block, though, so
> future travelers can see it.
> 
> P.S. You also have the option of calling posixpath.join() to do this.  Either
> one is fine with me.

Done.