[webkit-reviews] review granted: [Bug 33045] Add a script to test both the Python and Perl scripts : [Attachment 45723] Proposed patch 4

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 2 12:51:04 PST 2010


Eric Seidel <eric at webkit.org> has granted Chris Jerdonek
<chris.jerdonek at gmail.com>'s request for review:
Bug 33045: Add a script to test both the Python and Perl scripts
https://bugs.webkit.org/show_bug.cgi?id=33045

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
Looks fine.

A few python comments:
1.  Should really use a class to encapsulate the logic.  Then your __main__
just calls the class Foo().main()
2.  OptionParser (from optparse) is probably easier to use than getopt.

I would have probably made:
os.path.join(scripts_directory, 'test-webkit-perl')
a function, even though you only use it twice.

def script_path(script):
  return os.path.join(scripts_directory, script)

So yeah, this python could probably be made slightly better, but looks OK over
all.   Thank you for doing this!


More information about the webkit-reviews mailing list