Re: [PATCH v3] Produce more verbose error if cppi not found

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux