[webkit-reviews] review granted: [Bug 104279] [Mac] Add Build Phase to Check Headers for Inappropriate Macros (Platform.h macros) : [Attachment 178047] [PATCH] Part 1: Add script check-for-inappropriate-macros-in-external-headers

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 13 09:02:17 PST 2012


David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 104279: [Mac] Add Build Phase to Check Headers for Inappropriate Macros
(Platform.h macros)
https://bugs.webkit.org/show_bug.cgi?id=104279

Attachment 178047: [PATCH] Part 1: Add script
check-for-inappropriate-macros-in-external-headers
https://bugs.webkit.org/attachment.cgi?id=178047&action=review

------- Additional Comments from David Kilzer (:ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=178047&action=review


r=me, but please address Dan's comment and making sure to use the same ruby
binary in test-webkitruby when running tests.

Also, I think
Tools/Scripts/webkitruby/check-for-inappropriate-macros-in-external-headers-tes
ts/passing-expected.txt was probably renamed to pass-expected.txt, so it should
be removed from the commit.

> Tools/Scripts/test-webkit-scripts:31
> +"""Run unit tests of WebKit's Perl, Ruby, and Python scripts."""

Nit:  Should it be "Perl, Python and Ruby scripts" to keep them alphabetized? 
:)

> Tools/Scripts/test-webkit-scripts:80
> +	   print('Note: Perl, Ruby, and Python results appear separately
above.')

Ditto.

> Tools/Scripts/test-webkitruby:30
> +  puts %x{ ruby '#{test}' }

Does using "ruby" here imply that you're using the same ruby executable that
was used to launch this script?  If not, we should use the path the ruby binary
that was used to run this script (however you grab that in ruby; maybe "$0"?)
to make the behavior more predictable.


More information about the webkit-reviews mailing list