Summary: | webkitpy: lint code in webkitpy.layout_tests.models | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||
Component: | New Bugs | Assignee: | Dirk Pranke <dpranke> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, ojan, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dirk Pranke
2012-07-02 17:47:47 PDT
Created attachment 150506 [details]
Patch
Comment on attachment 150506 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150506&action=review > Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py:215 > + indices[i] += 1 # pylint: disable=W0631 > + for j in range(i + 1, r): # pylint: disable=W0631 What is the complaint here? Can we just fix it? > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:405 > + self.name = None # this is the path in the line itself > + self.path = None # this is the normpath of self.name Can you put a FIXME to rename these? original_path and normalize_path sgtm. > Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:115 > + super(FailureTimeout, self).__init__() IIRC, there's some issue with use super. I don't remember what it was, but everywhere else in webkitpy that I remember we just call the super-class directly. I think we probably should do that here for consistency sake at least. (In reply to comment #2) > (From update of attachment 150506 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150506&action=review > > > Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py:215 > > + indices[i] += 1 # pylint: disable=W0631 > > + for j in range(i + 1, r): # pylint: disable=W0631 > > What is the complaint here? Can we just fix it? > W0631 == "potentially uninitialized variable". I think this is a bug in the linter, because I don't see any way that i can be unused. I'll add a comment to that effect. > > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:405 > > + self.name = None # this is the path in the line itself > > + self.path = None # this is the normpath of self.name > > Can you put a FIXME to rename these? original_path and normalize_path sgtm. > Sure. > > Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:115 > > + super(FailureTimeout, self).__init__() > > IIRC, there's some issue with use super. I don't remember what it was, but everywhere else in webkitpy that I remember we just call the super-class directly. I think we probably should do that here for consistency sake at least. The issue w/ super has to do with multiple inheritance and __init__() methods that take different signatures; it's hard to use super() correctly if __init__() doesn't either (a) take no arguments or (b) take (..., *args, **kwargs). That said, I've found that using super is somewhat less brittle than calling the superclasses directly. I think you'll find plenty of uses of both at this point, and webkitpy.layout_tests mostly uses super(). Committed r122387: <http://trac.webkit.org/changeset/122387> |