[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review request reminder: kded-appmenu
From:       Alex Fiestas <afiestas () kde ! org>
Date:       2012-09-29 16:05:45
Message-ID: 1525439.TjmJQFtbd0 () monsterbad
[Download RAW message or body]

On Wednesday 26 September 2012 14:58:55 Cedric Bellegarde wrote:
> Hello,
> 
> i'm waiting for some code review on kded-appmenu module:
> https://projects.kde.org/projects/kdereview/kded-appmenu
> 
> Martin Gräßlin tells me to send a reminder here before commiting. I need to
> put this in kde-workspace before patching with:
> https://git.reviewboard.kde.org/r/104344/
> 
> As i'm now plasma-widget-menubar maintainer, i will release a compatible
> version as soon as this is released with KDE.
> 
> For libkappmenu, it's not needed inside KDE as dbusmenu will be deprecated
> by GMenuModel.
> 
> So, my next works will be:
> - start working on QMenuModel with Qt devs and Renato Araujo Oliveira Filho
> from Canonical
> - add support for QMenuModel in kded-appmenu
> 
> Regards,

I was wondering, can't the kded be loaded on demand? what if somebody doesn't 
care about appmenu?

Coding wise everything seems ok, only a few tips I apply to my code but you 
don't have to :p

-Don't use arguments names like "b" even if it is a bool :p
-Return asap, instead of having super huge blocks of conditionals.

As I said, personal "code clean" tips, don't change anything if you don't 
want.

Cheers and good work!
[prev in list] [next in list] [prev in thread] [next in thread]