[webkit-reviews] review denied: [Bug 34676] check-webkit-style: Switch from using getopt.getopt() to optparse.OptionsParser : [Attachment 52270] Proposed patch

[bugzilla-daemon at webkit.org]

Shinichiro Hamaji <hamaji at chromium.org> has denied Chris Jerdonek
<cjerdonek at webkit.org>'s request for review:
Bug 34676: check-webkit-style: Switch from using getopt.getopt() to
optparse.OptionsParser
https://bugs.webkit.org/show_bug.cgi?id=34676

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

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
This change looks good, I couldn't look the patch closely yet, though.

I'm not sure if --filter=? is better than --filter= . How about outputting the
list of available filters for all invalid filters such as "", "?",
"runtime/arrays", and etc?

Also, as this patch changes the behavior of --filter, I think it's nice to
mention this change in your ChangeLog.

> +	   sys.exit(2)

Why exit(2) is better than 1?

> This script can miss errors and does not substitute for code review.
> ERROR: option --min-confidence: invalid integer value: 'xxx'

Cannot we have one blank line before the "ERROR: ..." line?

Putting r- for now to reduce the number of patches in our review queue, but
I'll re-put r+ if I'm convinced.