Please review Snorenotify (Finish Incubating)

[Hannah von Reth]

Hi I'll answer inline even so I usually don't like it.And I fixed the clazy warnings.Thanks for the feedback :) 

> From: aleixpol at kde.org
> Date: Fri, 20 Nov 2015 01:51:13 +0100
> Subject: Re: Please review Snorenotify (Finish Incubating)
> To: vonreth at kde.org
> CC: kde-core-devel at kde.org
> 
> On Wed, Nov 18, 2015 at 11:48 PM, Hannah von Reth <vonreth at kde.org> wrote:
> > Hi there,
> >
> > Mario guided me until now through the incubation process and we think it is
> > time to move Snorenotify from playground to extragear.
> > Snorenotify is a notification framework supporting Linux, Windows and Mac
> > OSX.
> > It is not meant to replace Knotifications, it is more targeted on Qt
> > applications without dependency to the plasma desktop, so it might become a
> > backend for Knotifications.
> >
> > I guess you can find the most important information here
> > https://community.kde.org/Incubator/Projects/Snorenotify.
> >
> > Besides Snorenotify there is also Snoretoast, a sub project of Snorenotify,
> > https://quickgit.kde.org/?p=snoretoast.git.
> > Snoretoast  is a command line application and used within Snorenotify for
> > the Windows Toast notifications.
> > The application can only be build using the Microsoft compiler.
> >
> > So it would be great if Snorenotify could become a official KDE library and
> > maybe even a framework someday.
> > Currently it is used by Quassel and Tomahawk but hopefully more will start
> > to use it soon.
> >
> > So please review Snorenotify.
> >
> > If you find the idea of Snorenotify useful or you fancy notifications, like
> > I do, feel free to contribute ;)  or start to use Snorenotify.
> 
> Hi Hannah,
> I'm happy that you're decided to push this forward, I think it's a
> framework that could be definitely useful. In KDE Connect we'd like to
> be able to use it instead of KNotifications because it's leaner and
> more straightforward to our notifications use-case. As I said before,
> I already ported KDE Connect to use snore, nevertheless there's some
> issues like the API that I would suggest to review before committing
> to API and ABI stability.
> 
> Here's some thoughts:
> - It feels overly complex that the Notification object is passed
> around by value rather than by reference. For starters, being able to
> connect to the notification would be very handy. I work-arounded it by
> setting a hint with the relevant information, but I have the feeling
> the API would be slightly smoother.A Notification uses shared data and is not a QObject.The reason why I use shared data is that neither the backend nor the frontend/application knows when to delete the notification.
> - There's a hard dependency on QtWidgets from the very core of the
> framework. This requires applications that would use it to use
> QApplication. In the case of KDE Connect, for instance, the plan was
> to make it possible to have it in a QCoreApplication (or
> QGuiApplication at least). It's a daemon from which we consider that
> it's acceptable to show notifications but we don't really plan to open
> a dialog from there. Do you think it could/should be worked out?Hm I'll investigate what might be possible.
> - There's a daemon. We're kind of trying to get rid of daemons. When
> is it needed?It might be useful on mac and Windows to display freedesktop notifications but can be disabled -DBUILD_daemon=OFF
> - enum values are all-caps joined by underscore (e.g. GLOBAL_SETTING).
> Camel-case would be more Qt-friendly.Will do so for the next release.
> - Some methods use wid as a windows id. Please use QWindow whenever
> possible, it's being an issue in Plasma nowadays already.I'll have a look on it
> - Maybe we can port the internal logging infrastructure to just use
> QLoggingCategory?I'll try :)
> 
> Cheers! :)
> Aleix
 		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20151127/3c197a96/attachment.htm>