[webkit-reviews] review granted: [Bug 105915] [GTK] Add WebP image support : [Attachment 181160] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 105915: [GTK] Add WebP image support
https://bugs.webkit.org/show_bug.cgi?id=105915

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

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=181160&action=review


Thanks! Perhaps you could investigate a small change before landing:

> configure.ac:278
> +# Check for WEBP Image support
> +have_webp=no
> +AC_CHECK_HEADERS([webp/decode.h], [have_webp="yes"], [have_webp="no"])
> +if test "$have_webp" = "yes"; then
> +  WEBP_LIBS='-lwebp'
> +else
> +  AC_MSG_ERROR([WebP library (libwebp) not found])
> +fi
> +AC_SUBST([WEBP_LIBS])

I don't mean to be nitpicky, but I think you can probably simplify this a bit
by getting rid of the have_webp variable. That would be more useful if you were
actually using it later.

Instead I think you could just keep what you need here inline in the macro. For
instance, something like this should work:

AC_CHECK_HEADERS([webp/decode.h], [WEBP_LIBS="-lwebp'], AC_MSG_ERROR([WebP
library (libwebp) not found])
AC_SUBST([WEBP_LIBS])