Re: [RFC] qemu: convert DomainLogContext class to use GObject

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

 



On Tue, Mar 10, 2020 at 12:53:59 +0000, Daniel Berrange wrote:
> On Tue, Mar 10, 2020 at 01:48:39PM +0100, Peter Krempa wrote:
> > On Tue, Mar 10, 2020 at 12:42:47 +0000, Daniel Berrange wrote:
> > > On Tue, Mar 10, 2020 at 01:33:42PM +0100, Peter Krempa wrote:
> > > > On Tue, Mar 10, 2020 at 17:30:01 +0530, Gaurav Agrawal wrote:
> > > > > ---
> > > > >  src/qemu/qemu_domain.c  | 36 ++++++++++++++++++++----------------
> > > > >  src/qemu/qemu_domain.h  |  6 ++++--
> > > > >  src/qemu/qemu_process.c |  4 ++--
> > > > >  3 files changed, 26 insertions(+), 20 deletions(-)

[...]

> > > > 
> > > > There's no reason to break coding style rules in this kind of refactor
> > > > nor to make it inconsistent in span of 20 lines.
> > > 
> > > If you're refering to the use of underscores, then this is expected
> > > and indeed required because these method names are auto-generated
> > > by the GObject macros. This should only be the case for three
> > > specific methods (_init, _class_init and _get_type), with the rest
> > > all following normal style. See the virIdentity conversion for
> > > the prior example of this.
> > 
> > Yeah, I meant mainly that (also the newline after the type).
> > 
> > Can't we at least keep camel-case for the type itself?
> > 
> > 'qemuDomainLogContext_init'
> > 
> > That way at least looking for the symbol name will be less painful.
> 
> We had considered that in the original virIdentity conversion, but
> decided mixing two different naming styles in one identifier was
> even more ugly.

While ugly I still think that for greppability/searchability it's way
better to keep the object name using camel case. In the end as you've
pointed out it's just for the specific methods named above. Making it
look ugly is IMO less worse than missing something when trying to
understand how the code works.





[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