[webkit-reviews] review granted: [Bug 174551] [Scripts] Make svn-create-patch work better when called in sub directories : [Attachment 315573] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 174551: [Scripts] Make svn-create-patch work better when called in sub
directories
https://bugs.webkit.org/show_bug.cgi?id=174551

Attachment 315573: Patch

https://bugs.webkit.org/attachment.cgi?id=315573&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 315573
  --> https://bugs.webkit.org/attachment.cgi?id=315573
Patch

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

> Tools/Scripts/svn-create-patch:287
> +	   # svn may output explanitory lines describing more detail about a
file change

Spelling error here: explanatory

> Tools/Scripts/svn-create-patch:289
> +	   next if $line =~ / +>/;

I think you want a ^ at the start of the regular expression here, before the
space.

> Tools/Scripts/svn-create-patch:378
> +	   print STDERR "Performing `svn diff -r 0:${sourceRevision}
${escapedSourceFile} | tail -n +5`\n" if $verbose;

I would not use backquotes in STDERR. That’s perl syntax to mean "execute this
and turn the result into a string", but doesn’t seem right for the logging.
Maybe just normal \" quotes?

> Tools/Scripts/svn-create-patch:380
>	   print `svn diff -r 0:${sourceRevision} ${escapedSourceFile} | tail
-n +5`;

It’s surprising that we use print combined with backquotes here. Kind of a
peculiar way to run a command and let the output go to STDOUT as normal. I
think there are better perl idioms for this.