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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=131415





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

>>> Tools/Scripts/webkitpy/common/checkout/scm/git.py:-241
>>> -
>> 
>> Doesn't this break git?
> 
> This does not break git, because the git implementation of status_command() is not `git st`, but `git diff --name-status --no-renames HEAD`

I will move the previous SCM implementation into git.py, so as to avoid any changes when using the scm module with Git clients.

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

EDIT: SVNCheckoutStatus

>>> 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.
> 
> SVNStyleStatusCodeString

EDIT: SVNStatusLineItem

>> Tools/Scripts/webkitpy/common/checkout/scm/status.py:73
>> +    # Status convenience methods
> 
> Can't we use __getattr__ to implement these instead?

By this, I assume you mean that we should use the Dispatch pattern with __getattr__? We already have that, in essence, with the matches_code method. There is an alternative way to do this with setattr by defining a method like:

    def _define_matching_function(self, svn_status_code):
        def matching_function():
            return self.matches_status_code(svn_status_code)
        return matching_function

and then looping over all of the property constants and setting their snake_case representation to the function returned by _define_matching_function(). I find this hard to follow and unnecessarily complicated for a set of constants that will not be changing anytime soon.

Are you ok with keeping these properties explicitly defined?

>>> 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.
> 
> SVNStyleStatusCode

EDIT: SVNStatusCode

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