[Webkit-unassigned] [Bug 75825] webkit-patch land should automatically add svn:mime-type for .png files

[bugzilla-daemon at webkit.org]

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


Dirk Pranke <dpranke at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #138335|review?                     |review-
               Flag|                            |




--- Comment #11 from Dirk Pranke <dpranke at chromium.org>  2012-05-02 14:23:44 PST ---
(From update of attachment 138335)
View in context: https://bugs.webkit.org/attachment.cgi?id=138335&action=review

> Tools/Scripts/webkitpy/common/checksvnconfigfile.py:30
> +import sys

It looks like sys is unused in this file ...

> Tools/Scripts/webkitpy/common/checksvnconfigfile.py:33
> +def check(self, platform, fs):

Is 'platform' a PlatformInfo object? It looks like it is ... maybe this should take a host or a systemhost instead?

Also, some form of docstring would be good for this function. What is this function supposed to be checking for, and how should the return value be used?

> Tools/Scripts/webkitpy/common/checksvnconfigfile.py:72
> +    if platform.startswith('linux') or platform.startswith('darwin'):

This should probably be 

if platform.is_win():
  config_file_path = fs.join(os.environ ...
else:
  config_file_path = fs.join(fs.expand ...

> Tools/Scripts/webkitpy/style/checkers/png.py:45
>          self._platform = platform or sys.platform

Does the style checker have access to a host or a tool object? It should get the platform info from that ...

> Tools/Scripts/webkitpy/style/checkers/png.py:52
> +            errorcode = checksvnconfigfile.check(self, self._platform, self._fs)

Instead of returning an errorcode that is a bit mask, you should consider either returning two values in a tuple, or splitting this into two functions.

> Tools/Scripts/webkitpy/tool/steps/addsvnmimetypeforpng.py:36
> +        self._fs = FileSystem()

You can also get the filesystem from the tool.

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