[PATCH weston 02/11] compositor: add API to manage compositor instances

[Pekka Paalanen]

On Tue, 23 Jun 2015 18:19:38 -0700
"Jon A. Cruz" <jonc at osg.samsung.com> wrote:

> 
> Oh I forgot an item on the placement of comments.
> 
> 
> On 06/23/2015 04:31 PM, Jon A. Cruz wrote:
> > 
> > Minor doxygen note: the explicit "\brief" is not needed, and the brief
> > description should end with a '.' to allow the auto-brief to pick it up.
> > 
> > Overall looks good, but missing a few extra management calls. However
> > those seem applicable to patch 4/11.
> > 
> > 
> > On 06/22/2015 01:02 PM, Giulio Camuffo wrote:
> >> This commit adds three new exported functions:
> >> - weston_compositor_create() returns a new weston_compositor instance,
> >> initializing it as the now removed weston_compositor_init() did.
> >> - weston_compositor_exit(compositor) asks the compositor to tear
> >> down by calling the compositor's exit vfunc which is set by the
> >> libweston application.
> >> - weston_compositor_destroy(compositor) is called by the libweston
> >> application when tearing down the compositor. The compositor is destroyed
> >> and the memory freed.
> >> ---
> > 
> > Reviewed-by: Jon A. Cruz <jonc at osg.samsung.com>
> > 
> >>  src/compositor-drm.c      |   8 +--
> >>  src/compositor-fbdev.c    |   6 --
> >>  src/compositor-headless.c |   9 +--
> >>  src/compositor-rdp.c      |   4 --
> >>  src/compositor-wayland.c  |   9 +--
> >>  src/compositor-x11.c      |   5 +-
> >>  src/compositor.c          | 167 +++++++++++++++++++++++++++++++++-------------
> >>  src/compositor.h          |  14 +++-
> >>  8 files changed, 137 insertions(+), 85 deletions(-)

> >> diff --git a/src/compositor.c b/src/compositor.c
> >> index 4e91329..1924e6a 100644
> >> --- a/src/compositor.c
> >> +++ b/src/compositor.c
> >> @@ -4500,16 +4500,27 @@ timeline_key_binding_handler(struct weston_seat *seat, uint32_t time,
> >>  		weston_timeline_open(compositor);
> >>  }
> >>  
> >> -WL_EXPORT int
> >> -weston_compositor_init(struct weston_compositor *ec,
> >> -		       int *argc, char *argv[],
> >> -		       struct weston_config *config)
> 
> Aside from some details on the comments themselves (\brief, \ref...) it
> is more generally expected to see the doxygen comments in the .h file
> where the function is declared, instead of the .c file where it is
> implemented.

Seriously?

Personally I prefer documentation to reside with the implementation, so
that it is less easy to forget to update the docs. The compiler will
catch forgotten declaration, but not docs.

Maybe I picked it up from the Linux coding style.

Yes, I did merge your patch which does it differently than the whole rest
of existing Weston and Wayland. That was an oversight.


Thanks,
pq