[webkit-reviews] review denied: [Bug 53656] new-run-webkit-tests: split out thread stack logging code into a sharable module : [Attachment 81334] add log_traceback()

[bugzilla-daemon at webkit.org]

Mihai Parparita <mihaip at chromium.org> has denied Dirk Pranke
<dpranke at chromium.org>'s request for review:
Bug 53656: new-run-webkit-tests: split out thread stack logging code into a
sharable module
https://bugs.webkit.org/show_bug.cgi?id=53656

Attachment 81334: add log_traceback()
https://bugs.webkit.org/attachment.cgi?id=81334&action=review

------- Additional Comments from Mihai Parparita <mihaip at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=81334&action=review

> Tools/ChangeLog:14
> +	   * Scripts/webkitpy/common/system/stack_utils.py: Added.

webkitpy/common/thread may be a more appropriate location for this.

> Tools/Scripts/webkitpy/common/system/stack_utils.py:35
> +def log_wedged_thread(logger, name, id, msg=''):

Rename id to thread_id, so that it's more obvious what kind of ID this refers
to?

Also, nothing about this method seems specific to wedged threads,
log_thread_stack might be a better name.

> Tools/Scripts/webkitpy/common/system/stack_utils.py:45
> +def find_thread_stack(id):

Can this be called _find_thread_stack, to indicate that it's an internal-only
method.

> Tools/Scripts/webkitpy/common/system/stack_utils.py:62
> +def log_stack(logger, stack):

Assuming you don't need this in your other patches, can this be called
_log_stack?

> Tools/Scripts/webkitpy/common/system/stack_utils.py:63
> +    """Log a stack trace to log.error()."""

Doesn't actually log to log.error anymore.