[webkit-reviews] review granted: [Bug 33475] Create a unit-tested subroutine to parse a patch created by svn-create-patch : [Attachment 46744] Proposed patch

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Chris Jerdonek
<cjerdonek at webkit.org>'s request for review:
Bug 33475: Create a unit-tested subroutine to parse a patch created by
svn-create-patch
https://bugs.webkit.org/show_bug.cgi?id=33475

Attachment 46744: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=46744&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
> diff --git a/WebKitTools/Scripts/VCSUtils.pm
b/WebKitTools/Scripts/VCSUtils.pm
> +# Returns ($diffHashRef, $lastReadLine):
> +#   $diffHashRef:
> +#	 copiedFromPath: if a file copy, the path from which the file was
> +#			 copied. Otherwise, undefined.
> +#	 indexPath: the path in the "Index:" line.
> +#	 sourceRevision: the revision number of the source. This is the same
> +#			 as the revision number the file was copied from, in
> +#			 the case of a file copy.
> +#	 svnConvertedText: the diff converted to SVN format.
> +#   $line: the line last read from $fileHandle

"$line" should be "$lastReadLine" here.

> diff --git a/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl
b/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are
> +# met:
> +#
> +#	 * Redistributions of source code must retain the above copyright
> +# notice, this list of conditions and the following disclaimer.
> +#	 * Redistributions in binary form must reproduce the above
> +# copyright notice, this list of conditions and the following disclaimer
> +# in the documentation and/or other materials provided with the
> +# distribution.
> +#	 * Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> +# its contributors may be used to endorse or promote products derived
> +# from this software without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

This is the "old" BSD license.	I probably didn't catch this before, but you
should use the text from WebCore/LICENSE-APPLE instead if you have no
objections.

> +use strict;
> +use warnings;
> +
> +use Test::More;
> +use VCSUtils;

This doesn't require FindBin?  I guess it gets its include path from the script
that includes it.

> diff --git a/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parsePatch.pl
b/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parsePatch.pl
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are
> +# met:
> +#
> +#	 * Redistributions of source code must retain the above copyright
> +# notice, this list of conditions and the following disclaimer.
> +#	 * Redistributions in binary form must reproduce the above
> +# copyright notice, this list of conditions and the following disclaimer
> +# in the documentation and/or other materials provided with the
> +# distribution.
> +#	 * Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> +# its contributors may be used to endorse or promote products derived
> +# from this software without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Ditto for the license.

r=me