On Thu, May 15, 2025 at 05:41:40PM +0200, BALATON Zoltan wrote:
> On Thu, 15 May 2025, Zhao Liu wrote:
> > On Wed, May 14, 2025 at 06:24:03PM +0200, BALATON Zoltan wrote:
> > > Date: Wed, 14 May 2025 18:24:03 +0200
> > > From: BALATON Zoltan <bala...@eik.bme.hu>
> > > Subject: Re: [PATCH 8/9] target/s390x/kvm/pv: Consolidate
> > > OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> > >
> > > On Wed, 14 May 2025, Zhao Liu wrote:
> > > > > > +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest,
> > > > > > + s390_pv_guest,
> > > > > > + S390_PV_GUEST,
> > > > > > +
> > > > > > CONFIDENTIAL_GUEST_SUPPORT,
> > > > > > + { TYPE_USER_CREATABLE },
> > > > > > + { NULL })
> > > > >
> > > > > I'll note that existing callers of
> > > > > OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES
> > > > > happily ignore the line limit and put it into a single line.
> > > > >
> > > > > (which ends up looking better IMHO)
> > > >
> > > > Ok, I'll onor the existing conventions (which I'll apply to other
> > > > patches as well).
> > >
> > > There are two line limits. If something is clearer on one line you could
> > > exceed the normal 80 chars and put up to 90 chars on one line for which
> > > checkpatch will issue a warning that can be ignored for these cases. Over
> > > 90
> > > lines checkpatch will give an error and I think you should not ignore
> > > that.
> >
> > Thank you. This makes sense!
> >
> > > Maybe try to put as much on one line as possible instead of new line after
> > > each argument but without exceeding the 80 chars or if the whole line fits
> > > in 90 chars then use that. Or maybe do not indent second line under ( but
> > > with 4 spaces then you can fit it in two lines but lines over 90 chars are
> > > undesirable.
> >
> > HMM, I understand you mean:
> >
> > OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(S390PVGuest, s390_pv_guest,
> > S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT, { TYPE_USER_CREATABLE }, {
> > NULL })
> >
> > The second line is 82 chars and now I think this version is better.
>
> Yes and maybe can even fit in 80 chars if using { } instead of { NULL }.
Personally, once you have to break the line, I would be inclined
to have *nothing* after the '(' on the first line, and then break
at the list of interfaces. ie
OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(
S390PVGuest, s390_pv_guest, S390_PV_GUEST, CONFIDENTIAL_GUEST_SUPPORT,
{ TYPE_USER_CREATABLE }, { NULL })
With 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 :|
