[webkit-reviews] review granted: [Bug 33620] check-webkit-style: Move error() from cpp_style.py to checker.py : [Attachment 46538] Proposed patch

[bugzilla-daemon at webkit.org]

Shinichiro Hamaji <hamaji at chromium.org> has granted Chris Jerdonek
<chris.jerdonek at gmail.com>'s request for review:
Bug 33620: check-webkit-style: Move error() from cpp_style.py to checker.py
https://bugs.webkit.org/show_bug.cgi?id=33620

Attachment 46538: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=46538&action=review

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
Looks a sane code migration.

> -    def __init__(self, output_format, verbosity=1, filter=None,
> +    def __init__(self, output_format="emacs", verbosity=1, filter=None,

I think we can utilize DEFAULT_OUTPUT_FORMAT and DEFAULT_VERBOSITY?

> +	   if ((verbosity < 1) or (verbosity > 5)):

I think we usually don't have parentheses for the condition of if-statement,
but I don't care. Please remove them if you have no preference about this.

> +	   ProcessorOptions() # No ValueError: works

I slightly prefer to examine the constructed value if it has sane attributes.

> +	   # Spurious white space in filter rules.
> +	   (files, options) = parse(['--filter=+foo ,-bar'])
> +	   self.assertEquals(options.filter,
> +			     CategoryFilter(["-", "+whitespace", "+foo",
"-bar"]))

Thanks for adding this!