[webkit-reviews] review granted: [Bug 214947] REGRESSION (r264925): run-safari --debug no longer works : [Attachment 405535] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 29 18:04:00 PDT 2020


Darin Adler <darin at apple.com> has granted katherine_cheney at apple.com's request
for review:
Bug 214947: REGRESSION (r264925): run-safari --debug no longer works
https://bugs.webkit.org/show_bug.cgi?id=214947

Attachment 405535: Patch

https://bugs.webkit.org/attachment.cgi?id=405535&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 405535
  --> https://bugs.webkit.org/attachment.cgi?id=405535
Patch

This is fine:

- If these functions are linked but not called, you might want to put
ASSERT_NOT_REACHED in them to help people understand they are intentionally
left blank, or a comment, or both. I did that recently with some SPI that I had
to leave around for the same reason.

- It’s also nice to leave out the argument names; if we complied with "unused
argument" warnings, that would prevent them.

- Also, sometimes we move these things out of the SPI headers into an "old
Safari depends on these" block, often not even in a header file, to help make
sure no one accidentally adds new calls to the functions.

Not saying you need to do any of these things. Seems like you could land this
just like it is.


More information about the webkit-reviews mailing list