On Thu, Sep 19, 2019 at 12:05:23AM +0200, Fabiano Fidêncio wrote:
On Wed, Sep 18, 2019 at 9:57 PM Ján Tomko <jtomko(a)redhat.com>
wrote:
>
> On Wed, Sep 18, 2019 at 05:40:06PM +0200, Fabiano Fidêncio wrote:
> >On Wed, Sep 18, 2019 at 1:22 AM Fabiano Fidêncio <fidencio(a)redhat.com>
wrote:
> >>
> >> On Tue, Sep 17, 2019 at 8:17 PM Pavel Hrdina <phrdina(a)redhat.com>
wrote:
> >> >
> >> > On Tue, Sep 17, 2019 at 06:53:30PM +0200, Fabiano Fidêncio wrote:
> >> > > On Tue, Sep 17, 2019 at 5:22 PM Pavel Hrdina
<phrdina(a)redhat.com> wrote:
> >> > This is an intentional change to run syntax-check tests for dist
target
> >> > as well, but it might be possible to configure it somehow. Anyway, I
> >> > would rather prefer to run syntax-check when running ninja dist than
> >> > having it as a separate target.
> >>
> >> osinfo-db-tools has it, libosinfo has it, libvirt-jenkins-ci uses it.
> >> This is a pattern already being used and here I have a strong
> >> preference for not changing the pattern if we can easily adapt to it.
> >
> >Just for the record, we had a face-to-face discussion about this and
>
> Can you record the reasoning as well?
A few topics were brought up:
- It's closer to what different projects using meson are doing
(systemd was the example used);
- It runs on `ninja dist` without having to run an additional command
(`ninja syntax-check`);
- If the test is named 'syntax-check', the test can still be run as
`meson test syntax-check`;
Everything else is just about personal preference. And talking about
personal preference, I'd rather keep the syntax-check target, it looks
cleaner to me ... however, my personal preference is far from counting
something on "libvirt land" and I'm okay on adapting osinfo related
projects to whatever is decided here.
There are projects, though, which do not have any tests (gtk-vnc, for
example) and those will have to add a single test case to run
syntax-check in order to adapt to this decision. Up to the maintainers
to decide what to do ... but consistence, IMHO, is important.
A bit more to the reasoning behind dropping syntax-check target.
I was looking at a lot of meson-based projects [1] and none of the
projects that I've checked have syntax-check target and they are using
only the test target.
I was trying to search any usage of syntax-check but it looks like it's
a gnulib specific target, only few projects have this target defined in
Makefile.
Another argument from my side was that we require syntax-check and check
to be executed before posting patches to mailing list, having it under
single target would simplify things for new contributors and would align
more with meson community and projects.
Dan pointed out an issue with our CI that we would loose the separation
in our CI results, which can be solved by using 'suite' labels for tests
that we run, so for syntax-check we can use 'syntax' label and for unit
tests we can use 'unit' label, there can be multiple labels assigned to
each test and to run only a set of tests with a specific label we just
need to run these commands:
meson test --suite syntax
meson test --suite unit
This way we would still have the separation in our CI and contributors
could easily run `meson test` or `ninja test` to run everything.
Pavel
[1] <
https://mesonbuild.com/Users.html>