[PATCH weston 0/5] ivi-shell untangling

[Pekka Paalanen]

On Wed, 16 Mar 2016 09:18:03 +0000
"Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:

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

Cool, and pushed:
   1ddb8dd..32ca791  master -> master


Thanks,
pq

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160316/56310265/attachment.sig>