[Webkit-unassigned] [Bug 184039] webkitpy: Implement coredumpctl support on linux

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=184039

--- Comment #13 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 336776
  --> https://bugs.webkit.org/attachment.cgi?id=336776
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=336776&action=review

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:70
> +                                                   return_stderr=True)

Please write this on one line.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:79
> +                        r'Timestamp: .*(\d\d\d\d-\d+-\d+ \d+:\d+:\d+)', info):

Please write this on one line.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:90
> +            tf = tempfile.NamedTemporaryFile()

A more canonical name for this variable is temp or temp_file.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:92
> +                                            '--output=' + tf.name],

I take it coredumptctl only parses “output=“? Otherwise I would suggest passing the temporary file name as its own argument and avoid the string concatenation.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:93
> +                                           return_stderr=True,

This will be ignored when return_exit_code is True. So, please remove this.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:99
> +            return res

Please do not abbreviate names. This should be result.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:125
> +                log_directory, file_filter=match_filename)

Please write this on one line. We follow PEP-8 style for Python code plus the WebKit code style guidelines for variable and function naming.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:132
> +            crash_log, errors = self._get_trace_from_systemd(

Ditto.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:138
> +            ['c++filt', ], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

Ditto.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:144
> +            core_pattern = self._filesystem.join(

Ditto.

> Tools/Scripts/webkitpy/port/linux_get_crash_log_unittest.py:35
> +from webkitpy.common.system.executive_mock import MockExecutive

Please sort these from lines by their from clause. So, webkitpy.common should come before webkitpy.port.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180403/ef862235/attachment-0002.html>