[webkit-reviews] review granted: [Bug 116378] [GTK] [CMake] Add a "make dist" target : [Attachment 222393] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 116378: [GTK] [CMake] Add a "make dist" target
https://bugs.webkit.org/show_bug.cgi?id=116378

Attachment 222393: Patch
https://bugs.webkit.org/attachment.cgi?id=222393&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=222393&action=review


Great stuff!

> Tools/gtk/make-dist.py:82
> +	   yield (self.source_root, self.tarball_root)

Why use yield here? Doesn't look like we're returning something different ever.


> Tools/gtk/make-dist.py:126
> +	   if self.tarball_root[-1] != '/':
> +	       self.tarball_root = self.tarball_root + '/'
> +	   if self.tarball_root[0] != '/':
> +	       self.tarball_root = '/' + self.tarball_root

It was suggested before that we should use .startswith/.endswith, I think that
would be more readable indeed.

> Tools/gtk/make-dist.py:177
> +	   if parts[0][0] == "#":

parts[0].startswith()?

> Tools/gtk/manifest.txt:74
> +directory $build/Documentation/webkit2gtk/html Documentation/webkit2gtk/html

> +directory $build/Documentation/webkit2gtk/html Documentation/webkit2gtk/html

> +directory $build/Documentation/webkitgtk/tmpl Documentation/webkitgtk/tmpl
> +directory $build/Documentation/webkitgtk/tmpl Documentation/webkitgtk/tmpl

Shipping generated files has been frowned upon by some people before - mainly
because you can't make dist if you haven't made a full build with the
appropriate configuration options before that. I don't think it's a problem,
and I really like having pre-build documentation in the tarball, but just
thought I'd share this here.