On 6/17/19 6:40 PM, Andrea Bolognani wrote:
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 :)
Okay, I'll push only the first hunk after I've cleaned the commit
message a bit.
Thanks,
Michal
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
