[webkit-reviews] review denied: [Bug 191303] DumpRenderTree should report unknown options : [Attachment 358263] Added fallback else for unknown option
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jan 4 06:24:31 PST 2019
Frédéric Wang (:fredw) <fred.wang at free.fr> has denied darshan
<dkadu at igalia.com>'s request for review:
Bug 191303: DumpRenderTree should report unknown options
https://bugs.webkit.org/show_bug.cgi?id=191303
Attachment 358263: Added fallback else for unknown option
https://bugs.webkit.org/attachment.cgi?id=358263&action=review
--- Comment #4 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
Comment on attachment 358263
--> https://bugs.webkit.org/attachment.cgi?id=358263
Added fallback else for unknown option
View in context: https://bugs.webkit.org/attachment.cgi?id=358263&action=review
Thanks for the patch. Can you please rebase it (it seems it does not apply on
tip of tree) and add the associated ChangeLog?
Also, have you actually tested your patch with a currently unknown option? If
you find-grep tests, I'm sure some of them already use such unhandled options
and you might need to add them into TestOptions.cpp and update the
expectations. Also, I would be curious to see if the LOG_ERROR actually shows
up in the test output.
> Tools/DumpRenderTree/TestOptions.cpp:113
> + LOG_ERROR("Unknown option %s",key.c_str());
Missing space after comma (please run check-webkit-style before uploading the
patch)
> Tools/DumpRenderTree/TestOptions.cpp:114
> + break;
I think this break is not needed. We can just ignore the option (as we do now)
and continue to the next pair.
More information about the webkit-reviews
mailing list