[webkit-reviews] review granted: [Bug 208970] [GTK] Use #!/usr/bin/python3 shebang to run generate-gtkdoc : [Attachment 393330] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Michael Catanzaro
<mcatanzaro at gnome.org>'s request for review:
Bug 208970: [GTK] Use #!/usr/bin/python3 shebang to run generate-gtkdoc
https://bugs.webkit.org/show_bug.cgi?id=208970

Attachment 393330: Patch

https://bugs.webkit.org/attachment.cgi?id=393330&action=review




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

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

This change looks good.

> Tools/ChangeLog:10
> +	   Fedora and Ubuntu are both patching generate-gtkdoc to use
#!/usr/bin/python3 rather than

This is ok as-is. No change is needed. The optimal ChangeLog would move all the
text in this line and later ABOVE the file name so as to be the description of
the change because:

1. There is only one file changed in this patch so there's no need for per file
comments.

2. Per file/function comments should be concise, no longer than a single
paragraph. They are meant to provide more fine grain descriptions of the
charges in the patch.

> Tools/ChangeLog:13
> +	   #!/usr/bin/env and /usr/bin/python are both banned in Fedora
packages. I guess Ubuntu

This is ok as-is. No change is needed. A slightly better description would
reference the official documentation by URL, along with date accessed (if the
URL doesn't look permanent - encode a date or version) and optionally include 
an excerpt from it if the URL does not look permanent. In other words, a
slightly better description provides irrefutable proof why the change was made
in a way that a third party in the future can verify the claim.