On Thu, Sep 19, 2019 at 12:09:23PM +0200, Ján Tomko wrote: > On Thu, Sep 19, 2019 at 09:36:35AM +0100, Daniel P. Berrangé wrote: > > 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); > > Not a fan of using systemd as an example O:-) > > > > > - 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. > > > > I thought that was exactly the point of keeping the target separate - > not to run the tests meant for patch submissions. For a downstream > example, I remember us not caring that much for exact spacing or failing > copyright year check on our RHEL branches. There is that, but if we look at the kind of things our checks do, a non-trivial number of checks are verging on code correctness rather than merely style check. So from that POV, I think running these checks downstream is actally desirable in general. They can still opt-out though, if we put the syntax check as suite test suite from meson's POV. > (For example QEMU has these checks separate and only ran on the new > patch, not the whole codebase) QEMU is a good example of why that approach is a terrible idea. A huge portion of QEMU's codebase doesn't comply with the style checks as no one ever bothers to clean up after new style rules are added. As a result people submitting patches find themselves having to fix problems with existing code in order for their patch to succeed. So you get patches submitted which mix style and new features in the same patch. 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
