[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 #14 from Thibault Saunier <tsaunier at gnome.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

I did not change the line length and kept following PEP8, if you really think we should not follow the 79 line length, I could change.

>> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:70
>> +                                                   return_stderr=True)
> 
> Please write this on one line.

I am following pep8 as advised here: https://webkit.org/code-style-guidelines/#python

pep8 is pretty clear about line length: https://www.python.org/dev/peps/pep-0008/#maximum-line-length

Previous code was not following the proper code style but I did not fix it to avoid filling my patch with unrelated changes.

>> 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.

Done.

>> 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.

Done, it also supports `--output outfilename`

>> 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.

Done.

>> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:99
>> +            return res
> 
> Please do not abbreviate names. This should be result.

Fixed (just returned).

>> 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.

Well, I just followed what was there. (fixed all)

-- 
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/74b4b41f/attachment-0002.html>