On Wed, 16 Mar 2016 08:16:38 +0000 "Ucan, Emre (ADITG/SW1)" <[email protected]> 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:[email protected]] > > Sent: Dienstag, 15. März 2016 16:40 > > To: [email protected] > > Cc: Ucan, Emre (ADITG/SW1); wataru_natsume; Pekka Paalanen > > Subject: [PATCH weston 0/5] ivi-shell untangling > > > > From: Pekka Paalanen <[email protected]> > > > > 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 >
pgpiQn8qzvM71.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
