[webkit-reviews] review denied: [Bug 57662] new-run-webkit-tests: fix feature detection, skipped platform lists on mac : [Attachment 87922] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 1 16:32:39 PDT 2011


Adam Barth <abarth at webkit.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 57662: new-run-webkit-tests: fix feature detection, skipped platform lists
on mac
https://bugs.webkit.org/show_bug.cgi?id=57662

Attachment 87922: Patch
https://bugs.webkit.org/attachment.cgi?id=87922&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=87922&action=review

> Tools/Scripts/webkitpy/layout_tests/port/mac.py:134
> +    def _path_to_webcore_library(self):
> +	   return self._build_path('WebCore.framework/Versions/A/WebCore')

This method does not appear to be used in this patch.

> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:186
> +	   entries = self._filesystem.glob(self._webkit_baseline_path('*'))

Doesn't this return a super huge list?	This doesn't seem like the right
algorithm.  You want to enumerate the directories and then ask whether they're
in the baseline search path, not enumerate all the things in those directories.


> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:187
> +	   platform_prefix = self.name().split('-', 1)[0]

This should be a helpful function instead of munging the string here.


More information about the webkit-reviews mailing list