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 <[email protected]> Best regards Emre Ucan Software Group I (ADITG/SW1) Tel. +49 5121 49 6937 > -----Original Message----- > From: Pekka Paalanen [mailto:[email protected]] > Sent: Mittwoch, 16. März 2016 10:10 > To: Ucan, Emre (ADITG/SW1) > Cc: [email protected]; wataru_natsume > Subject: Re: [PATCH weston 0/5] ivi-shell untangling > > 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 > > _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
