[Webkit-unassigned] [Bug 75824] check-webkit-style should warn about missing svn:mime-type for png files

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from David Levin <levin at chromium.org>  2012-03-09 09:00:34 PST ---
(From update of attachment 131040)
View in context: https://bugs.webkit.org/attachment.cgi?id=131040&action=review

I have some concerns/questions but this may be approvable if someone else feels confident in it (or with appropriate explanations).

> Tools/Scripts/webkitpy/style/checker.py:498
> +            return False

Why was this needed for png but no other type?

> Tools/Scripts/webkitpy/style/checkers/png.py:29
> +import sys

sort

> Tools/Scripts/webkitpy/style/checkers/png.py:50
> +                print "There is no ~/.subversion/config"

I don't think we print from these checkers.

Also this won't get flagged as an error at all. Isn't that incorrect given the current code?

> Tools/Scripts/webkitpy/style/checkers/png.py:51
> +                #return 1

Please remove the commented out code.

> Tools/Scripts/webkitpy/style/checkers/png.py:70
> +        if errorstr != "":

Just

if errorstr:

> Tools/Scripts/webkitpy/style/checkers/png.py:71
> +            self.handle_style_error(0, 'image/png', 5, errorstr)

I thought it would look at the diff to determine this. I thought you could set the prop without it being in the config file.

Hopefully someone more familiar with this will look at this part of the code.

> Tools/Scripts/webkitpy/style/checkers/png_unittest.py:41
> +        self.assertEquals(checker.handle_style_error, mock_handle_style_error)

This doesn't look like it verifies that the checker actually finds the error.

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