[Webkit-unassigned] [Bug 131415] Improve support for SVN statuses in scm.py

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 22 10:11:42 PDT 2014


--- Comment #5 from Matthew Hanson <matthew_hanson at apple.com>  2014-05-22 10:12:05 PST ---
(From update of attachment 231854)
View in context: https://bugs.webkit.org/attachment.cgi?id=231854&action=review

I will send a v2 with Ryosuke's feedback incorporated.

> Tools/Scripts/webkitpy/common/checkout/scm/status.py:4
> +class CheckoutStatus(object):


>> Tools/Scripts/webkitpy/common/checkout/scm/status.py:6
>> +        self.file_statuses = [FileStatus(line, checkout_root) for line in text.split("\n") if line]
> We usually prefix private variables with _.

Noted. I will add the _ prefix.

>> Tools/Scripts/webkitpy/common/checkout/scm/status.py:45
>> +class FileStatus(object):
> FileStatus is an extremely generic name.  Please give it a more descriptive name.


>> Tools/Scripts/webkitpy/common/checkout/scm/status.py:175
>> +class Code(object):
> Code is an extremely generic class name.  Please give it a more descriptive name.


>> Tools/Scripts/webkitpy/common/checkout/scm/status.py:194
>> +                            "in its place.")
> We don't align text line this with indentations.  Please indent each line with exactly 4 spaces.
> Also, these descriptions are split into so many lines.  We can probably fit this into much fewer lines.

Will do.

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list