Re: [PATCH 4/8] tests: Use virAsprintf() to build titles

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

 



On Wed, 2019-03-13 at 10:32 +0100, Peter Krempa wrote:
> On Wed, Mar 13, 2019 at 10:25:16 +0100, Andrea Bolognani wrote:
> > On Wed, 2019-03-13 at 09:48 +0100, Peter Krempa wrote:
> > > On Thu, Mar 07, 2019 at 16:44:33 +0100, Andrea Bolognani wrote:
> > [...]
> > > >  #define DO_TEST(arch, name) \
> > > >      do { \
> > > > +        VIR_AUTOFREE(char *) title = NULL; \
> > > > +        VIR_AUTOFREE(char *) copyTitle = NULL; \
> > > > +        if (virAsprintf(&title, "%s (%s)", name, arch) < 0 || \
> > > > +            virAsprintf(&copyTitle, "copy %s (%s)", name, arch) < 0) { \
> > > > +            return -EXIT_FAILURE; \
> > > 
> > > Coding style. Single-line body.
> > 
> > There are multiple conditions with the same indentation, so per the
> > coding guidelines[1] the curly braces are required.
> > 
> > Honesly, we should really give clang-format or whatever similar tool
> > a serious go and just start enforcing that code needs to be filtered
> > through it before being merged. Having humans worry about this kind
> > of nonsense is such an utter waste of time.
> > 
> > 
> > [1] https://libvirt.org/hacking.html#curly_braces, third example.
> 
> Hmm, interresting. In this particular instance we are pretty much always
> breaking the style though. Majority of multi-line conditions with a
> single line body which I've encountered don't have the block.

Yeah, none of the style rules that doesn't have a corresponding
syntax-check rule is really enforced consistently, which is why we
should consider flipping the workflow and just have a tool reformat
the code for us in the first place.

-- 
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