[webkit-reviews] review granted: [Bug 54701] pathRelativeToSVNRepositoryRootForPath needs to handle svn+ssh URLs : [Attachment 84506] Take 2

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Matt Lilek (pewtermoose)
<mlilek at apple.com>'s request for review:
Bug 54701: pathRelativeToSVNRepositoryRootForPath needs to handle svn+ssh URLs
https://bugs.webkit.org/show_bug.cgi?id=54701

Attachment 84506: Take 2
https://bugs.webkit.org/attachment.cgi?id=84506&action=review

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=84506&action=review

Thank you Matt for separating out the parsing logic and adding a unit test!

> Tools/ChangeLog:8
> +	   * Scripts/VCSUtils.pm:

By convention we usually document the functions we've added here because
prepare-ChangeLog doesn't know how to generate this information for Perl
scripts. (We should teach prepare-ChangeLog to do so). See
<http://trac.webkit.org/changeset/77028> and
<http://trac.webkit.org/changeset/52692> for examples of the format we use.

>
Tools/Scripts/webkitperl/VCSUtils_unittest/parseSvnPathFromRepositoryRoot.pl:15

> +#	 * Neither the name of Research In Motion Limited nor the names of

Nit: "Research In Motion Limited" => "Apple Inc."

>
Tools/Scripts/webkitperl/VCSUtils_unittest/parseSvnPathFromRepositoryRoot.pl:53

> +my $svnInfo_http = "Path: update-webkit
> +Name: update-webkit
> +URL:
http://svn.webkit.org/repository/webkit/trunk/Tools/Scripts/update-webkit
> +Repository Root: http://svn.webkit.org/repository/webkit
> +Repository UUID: 268f45cc-cd09-0410-ab3c-d52691b4dbfc
> +Revision: 80194
> +Node Kind: file
> +Schedule: normal
> +Last Changed Author: dpranke\@chromium.org
> +Last Changed Rev: 78520
> +Last Changed Date: 2011-02-14 15:59:48 -0800 (Mon, 14 Feb 2011)
> +Text Last Updated: 2011-03-02 18:26:56 -0800 (Wed, 02 Mar 2011)
> +Checksum: 7639a3c28d7d1828af0aad8587cf9e6b
> +
> +";

So that we don't have to escape the '@' we should use non-interpolated
"here-document" syntax:

my $svnInfo = <<'END';
Path: update-webkit
Name: update-webkit
...
END

I explicitly wrote the ';' on the first line, did not put a space between "<<"
and 'END', and single-quote END in the aforementioned example (that is, these
are not typos). For more information on "here-document" syntax see
<http://perldoc.perl.org/perlop.html>. You can also see examples of
here-document syntax in some of the existing unit tests, say
parseGitDiffHeader.pl.

>
Tools/Scripts/webkitperl/VCSUtils_unittest/parseSvnPathFromRepositoryRoot.pl:72

> +my $svnInfo_svnPlusSSH = "Path: update-webkit
> +Name: update-webkit
> +URL:
svn+ssh://username\@svn.webkit.org/repository/webkit/trunk/Tools/Scripts/update
-webkit
> +Repository Root: svn+ssh://username\@svn.webkit.org/repository/webkit
> +Repository UUID: 268f45cc-cd09-0410-ab3c-d52691b4dbfc
> +Revision: 80194
> +Node Kind: file
> +Schedule: normal
> +Last Changed Author: dpranke\@chromium.org
> +Last Changed Rev: 78520
> +Last Changed Date: 2011-02-14 15:59:48 -0800 (Mon, 14 Feb 2011)
> +Text Last Updated: 2011-03-02 18:26:56 -0800 (Wed, 02 Mar 2011)
> +Checksum: 7639a3c28d7d1828af0aad8587cf9e6b
> +
> +";

Ditto.