[PATCH weston 0/5] ivi-shell untangling

[Ucan, Emre (ADITG/SW1)]

Hi Pekka,

I agree with you that we can let it like this for now. Because as you said we will change ivi-layout.c a lot.

Reviewed-by: Emre Ucan <eucan at de.adit-jv.com>

Best regards

Emre Ucan
Software Group I (ADITG/SW1)

Tel. +49 5121 49 6937

> -----Original Message-----
> From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
> Sent: Mittwoch, 16. März 2016 10:10
> To: Ucan, Emre (ADITG/SW1)
> Cc: wayland-devel at lists.freedesktop.org; wataru_natsume
> Subject: Re: [PATCH weston 0/5] ivi-shell untangling
> 
> On Wed, 16 Mar 2016 08:16:38 +0000
> "Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:
> 
> > Hi Pekka-san,
> >
> > Basically, it looks good. But I have one question:
> >
> > Would it not be better to merge ivi-shell.c and ivi-layout.c together,
> > if they are the some component ?
> >
> > We can then merge also ivi_layout struct to ivi_shell struct, and
> > ivi_layout_surface struct to ivi_shell_surface struct etc.
> >
> > I do not see any disadvantage not to do so.
> 
> Hi Emre,
> 
> I'd like to avoid that for now.
> 
> ivi-shell.c implements/forwards the client-visible protocol interface
> ivi_application and acts on wl_surface content updates. It is a fairly short and
> nice bit of code, while the code in ivi-layout.c is IMHO in need of a complete
> make-over and deals with very different things than ivi-shell.c.
> 
> Besides, merging .c files together has no value in itself. It is true that they are
> part of the same plugin, but as they deal with different aspects, I think it
> makes sense to keep them separate until there is a better reason to merge
> them. ivi-layout.c is not too short to begin with. ;-)
> 
> Weston core is composed of several .c files, too, and desktop-shell/shell.c
> could really use splitting up logically into separate files.
> 
> Merging the structs might be nice for a more clear allocation/freeing
> patterns, but maybe that could be clarified also while keeping things split. I
> also don't like the get_instance() in ivi-layout.c because it pulls out a pointer
> from thin air (a global) - that could be ridden with at the same time.
> 
> I still think ivi-shell.c and ivi-layout.c do different enough things to keep them
> separate, but that could change as cleaning up ivi-layout.c progresses.
> 
> Can I have your Reviewed-by for landing the remaining parts of this series?
> 
> It's just the last patch that is going towards more separation between ivi-
> shell.c and ivi-layout.c, while the earlier ones bring them closer together.
> 
> 
> Thanks,
> pq
> 
> > > -----Original Message-----
> > > From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
> > > Sent: Dienstag, 15. März 2016 16:40
> > > To: wayland-devel at lists.freedesktop.org
> > > Cc: Ucan, Emre (ADITG/SW1); wataru_natsume; Pekka Paalanen
> > > Subject: [PATCH weston 0/5] ivi-shell untangling
> > >
> > > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > >
> > > Hi,
> > >
> > > here are few clean-ups for ivi-shell, and removal of a signal in
> > > favour of a direct unconditional call. I'm also extracting the
> > > ivi-shell facing API part out of ivi-layout-private.h to make it a
> > > little more readable.
> > >
> > >
> > > Thanks,
> > > pq
> > >
> > > Pekka Paalanen (5):
> > >   ivi-shell: include config.h in ivi-layout-transition.c
> > >   ivi-shell: add include guards on ivi-shell.h
> > >   ivi-shell: call send_surface_send_configure() directly
> > >   ivi-shell: remove configured signal from ivi-layout
> > >   ivi-shell: introduce ivi-layout-shell.h
> > >
> > >  Makefile.am                       |  1 +
> > >  ivi-shell/ivi-layout-private.h    | 24 +-------------
> > >  ivi-shell/ivi-layout-shell.h      | 67
> > > +++++++++++++++++++++++++++++++++++++++
> > >  ivi-shell/ivi-layout-transition.c |  7 +++-
> > >  ivi-shell/ivi-layout.c            | 38 +++++++---------------
> > >  ivi-shell/ivi-shell.c             | 42 ++++++++++--------------
> > >  ivi-shell/ivi-shell.h             |  9 ++++++
> > >  7 files changed, 113 insertions(+), 75 deletions(-)  create mode
> > > 100644 ivi- shell/ivi-layout-shell.h
> > >
> > > --
> > > 2.4.10
> >