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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 23 22:58:20 PDT 2014


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





--- Comment #8 from Ryosuke Niwa <rniwa at webkit.org>  2014-05-23 22:58:42 PST ---
(From update of attachment 231979)
View in context: https://bugs.webkit.org/attachment.cgi?id=231979&action=review

(In reply to comment #6)
>
> 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?

Why do they need to be actual properties?  Can't they just be method calls in call sites?
If we did that, then we can simply iterate through the list of status codes and check whether it matches or not.

Alternatively, why can't we just export status objects and let call sites manually call matches_status_code?

I don't like the fact we're listing every status code name almost thrice in SVNCheckoutStatus, SVNStatusLineItem, and for status code objects themselves.

> Tools/Scripts/webkitpy/common/checkout/scm/svn_status.py:78
> +    def _define_matching_function(self, svn_status_code):
> +        def matching_function():
> +            return self.matches_status_code(svn_status_code)
> +        return matching_function

Where is this used?

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