On Mon, Sep 30, 2019 at 04:05:57PM +0200, Andrea Bolognani wrote:
On Mon, 2019-09-30 at 13:41 +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 30, 2019 at 02:18:17PM +0200, Andrea Bolognani wrote:
> > On Mon, 2019-09-30 at 13:56 +0200, Pavel Hrdina wrote:
> > > Agreed, for now let's keep all the wrappers but eventually we can
remove
> > > them to make the code cleaner.
> >
> > Note that we should be able to use VIR_AUTOPTR() instead of
> > g_autoptr() even for objects, by simply registering the same
> > GLib-provided free function with our own macros.
> >
> > That way we could keep using VIR_AUTO* everywhere and then, when
> > we're ready, mechanically switch everything to g_auto* in one fell
> > swoop, without having any point in time where the two styles are
> > coexisting.
>
> That creates an even bigger conversion later. Such big conversions
> cause more pain for backports, than doing an incremental conversion
> at appropriate times.
>
> Converting everything to g_autofree right now doesn't give style
> consistency as we'd still be matching VIR_ALLOC + g_autofree,
> so I don't see a benefit to a big conversion in 1 step.
>
> Incrementally converting VIR_ALLOC + VIR_AUTOFREE at the same
> time, makes more sense stylewise, as then within the scope of a
> single method we'd be consistent.
I see your point about backports being more painful when you have
a bunch of unrelated changes mixed in, but I would still prefer if
we converted everything at once and at the same time introduced a
suitable syntax-check rule preventing more instances of whatever
function we just removed all callers of from creeping back in, or
actually just dropping the function altogether.
Doing the conversion incrementally will IMHO result in dragging it
for much longer, causing more pain in the long run than ripping the
bandaid would.
There's really not any significant real world pain from mixing the
two styles. It is visually distasteful but doesn't cause any functional
problems at runtime, nor complexity for maintainers. A large conversion
over the whole codebase does cause very significant pain in conflicts
for anyone cherry picking patches. That is just not a net win overall.
I'll take visually mixed styles any day over creating patch conflicts
in backports.
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 :|