On Mon, 2019-06-17 at 18:21 +0200, Michal Privoznik wrote:
> It's fairly easy (especially for new contributors) to not spot
> the 'cppi not installed' line in the syntax-check output. Turn it
> into a banner that is more visible
Good so far.
> and at the same time add it as
> a build dependency. Unfortunately, RHEL doesn't ship cppi so we
> can add the dependency only for Fedora.
Depending on the outcome of the discussion below, this part might
need to be dropped.
> Since it's v1 this has effectively became code copied over from
> Andrea's review suggestions.
I don't feel like this remark belongs in the git history.
[...]
> syntax-check: spacing-check test-wrap-argv \
> prohibit-duplicate-header mock-noinline group-qemu-caps \
> header-ifdef
> + @if ! cppi --version >/dev/null 2>&1; then \
> + echo "*****************************************************" >&2; \
> + echo "* cppi not installed, some checks have been skipped *" >&2; \
> + echo "*****************************************************" >&2; \
> + fi
> endif
ACK to this hunk.
[...]
> +# For 'make syntax-check'
> +%if 0%{?fedora}
> +BuildRequires: cppi
> +%endif
CC'ing Dan and Martin, who argued for having this, and Pavel, who
argued against and made me switch sides. Let's see what ends up
being its fate :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
