On Thu, Sep 19, 2019 at 10:20:04AM +0200, Pavel Hrdina wrote: > 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@xxxxxxxxxx> 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@xxxxxxxxxx> wrote: > > > >> > > > >> On Tue, Sep 17, 2019 at 8:17 PM Pavel Hrdina <phrdina@xxxxxxxxxx> 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@xxxxxxxxxx> 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`); I think this is the compelling reason that I didn't understand before. "ninja dist" is *NOT* the same as "make dist" from autoconf world. It is in fact equivalent to 'make distcheck'. If we have syntx-check as a separate target, then we're loosing a safety check at time of release. > > - 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. Yes, that's a usable approach. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
