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

[bugzilla-daemon at webkit.org]

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

Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #337700|review?                     |review+
              Flags|                            |

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

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

Thank you Thibault for iterating on this patch. The patch looks sane to me.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:77
> +                for datestr in re.findall(r'Timestamp: .*(\d\d\d\d-\d+-\d+ \d+:\d+:\d+)', info):

I do not see the need to abbreviate the word “string” and “date string” is two words. For your consideration I suggest that we name this variable timestamp given that the we are matching against the extracted “Timestamp” values. On another note, we can simplify this regular expression by using repetition syntax, \d{x}, where x is the number of repetitions: ‘Timestamp .*(\d{4}-\d+-\d+ \d+:\d+:\d+’. We could simplify this further using repetition syntax and non-capture groups though I am unclear whether that would hurt readability.

> Tools/Scripts/webkitpy/port/linux_get_crash_log.py:107
> +        # Poor man which, ignore any failure.

For your consideration I suggest using manual page syntax when referencing another program name. So, “which”  => “which(1)”. This makes it straightforward for a reader to identify a command from an English word.

-- 
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/20180412/0c6bb768/attachment-0002.html>