[webkit-reviews] review granted: [Bug 233570] [Flatpak SDK] Update to FDO 21.08.6 release : [Attachment 445290] Patch

[bugzilla-daemon at webkit.org]

Adrian Perez <aperez at igalia.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 233570: [Flatpak SDK] Update to FDO 21.08.6 release
https://bugs.webkit.org/show_bug.cgi?id=233570

Attachment 445290: Patch

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




--- Comment #2 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 445290
  --> https://bugs.webkit.org/attachment.cgi?id=445290
Patch

Patch LGTM, I wrote a couple of minor comments, though.

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

> Tools/buildstream/elements/sdk/gst-libav.bst:26
> +	 PAGER=this-is-not-a-pager gst-inspect-1.0 libav

I suppose using PAGER='' to leave it empty may not work if gst-inspect-1.0 uses
some hardcoded default like trying to search for “less“… have you tried that?
Another option could be PAGER=cat but honestly the chance of something called
“this-is-not-a-pager“ being installed and executable is virtually zero so let's
just leave it as-is ��️

> Tools/buildstream/elements/sdk/gst-plugins-base.bst:-45
> -

Is there any reason to remove this empty line? Maybe we can leave it to avoid
bringing noise into the diff—and also I find the YAML more legible keeping this
line.

> Tools/buildstream/elements/sdk/gstreamer.bst:-9
> -

Same for this empty line here.

> Tools/buildstream/elements/sdk/gstreamer.bst:-34
> -

…and this one here.