Bug 63564

Created attachment 98979 [details] Patch

Comment on attachment 98979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98979&action=review > Websites/webkit.org/coding/coding-style.html:606 > +<li>On functions with more than one parameter, prefer enums to bools if callers are likely to be passing constants. This rule applies to functions with one parameter too. The number of parameters is not the key. Maybe the reason you though it depended on the number of parameters is that it does not apply to a setXXX function where the name of the function makes clear what the boolean is. I also think the guideline should explain the reason. The code below does allude to the reason this matters, but it’s worth saying that the named constants are much easier to read at the call site.

Comment on attachment 98979 [details] Patch review- because I’d like to remove the mistaken “more than one parameter”

Created attachment 99083 [details] Patch

Comment on attachment 99083 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99083&action=review Thanks. Getting better. > Websites/webkit.org/coding/coding-style.html:607 > +passing constants, since named constants are easier to read on the call site. Should be "at the call site" rather than "on the call site". > Websites/webkit.org/coding/coding-style.html:608 > +The only exception would be on setters, where the name of the function Instead of "The only exception would be on setters," I would say, "An exception to this rule is a setter function," > Websites/webkit.org/coding/coding-style.html:613 > +doSomething(something, AllowFooBar); > +paintTextWithShadows(context, ..., textStrokeWidth > 0, isHorizontal()); The right example should include a setter that takes a boolean. > Websites/webkit.org/coding/coding-style.html:618 > +doSomething(something, false); The wrong example should include a setter that takes an enum.

Created attachment 99107 [details] Patch

(In reply to comment #5) > > Should be "at the call site" rather than "on the call site". > Instead of "The only exception would be on setters," I would say, "An exception to this rule is a setter function," > The right example should include a setter that takes a boolean. > The wrong example should include a setter that takes an enum. > Fixed everything. Thanks.

The commit-queue encountered the following flaky tests while processing attachment 99107 [details]: http/tests/media/reload-after-dialog.html bug 59901 (authors: annacc@chromium.org, eric.carlson@apple.com, jamesr@chromium.org, and vrk@chromium.org) The commit-queue is continuing to process your patch.

Comment on attachment 99107 [details] Patch Clearing flags on attachment: 99107 Committed r90027: <http://trac.webkit.org/changeset/90027>

All reviewed patches have been landed. Closing bug.