[PATCH weston 0/5] ivi-shell untangling

[Pekka Paalanen]

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/229a5fc8/attachment.sig>