[webkit-reviews] review denied: [Bug 16441] ER: support for saving js stack trace to file : [Attachment 18857] enable webkit to save javascript trace from inspector window

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 13 22:51:04 PDT 2008


Adam Roben (aroben) <aroben at apple.com> has denied 's request for review:
Bug 16441: ER: support for saving js stack trace to file
http://bugs.webkit.org/show_bug.cgi?id=16441

Attachment 18857: enable webkit to save javascript trace from inspector window
http://bugs.webkit.org/attachment.cgi?id=18857&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
Thanks for the patch!

Patches need to be marked r? in order to be reviewed.

You need to explain the changes you made in your ChangeLog. You should also set
the CHANGE_LOG_NAME and CHANGE_LOG_EMAIL_ADDRESS environment variables so that
prepare-ChangeLog will generate a correct ChangeLog header.

This patch puts a lot of CoreFoundation-specific code into a cross-platform
source file, InspectorController.cpp. We should be using WebCore's
cross-platform functions/types instead.

We don't normally put in comments referencing the bug report that motivated the
addition of the code.

I think this feature would make a lot more sense in the Inspector if we had an
integrated JavaScript debugger (see bug 17134). Until then it makes more sense
in Drosera.

We probably want to present a file picker to the user so that they can choose
where to save the file instead of saving to an arbitrary file in a Mac-specific
location (we want this to work on non-Mac platforms as well).

Please read our code style guidelines at
<http://webkit.org/coding/coding-style.html> and make sure your code conforms
to them.


More information about the webkit-reviews mailing list