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

List:       kde-devel
Subject:    Re: Patch for KTeaTime
From:       Daniel Teske <teske () squorn ! de>
Date:       2007-09-05 21:38:43
Message-ID: 200709052338.43738.teske () squorn ! de
[Download RAW message or body]

Hi Stefan,

this patch looks very good. Good job and congratulations. You mentioned that you are \
very new to C** development. So I'm curious how did you learn C++, because obviously \
you got a lot of things right.

I think this patch as is, could go (almost) in. So doyou want me to sumbit it, or \
would want to apply for an svn account?

Anyway the last few comments:
I still think in the long term kteatime would be more usefull as a plasmoid.

And on the code:
Slots are normal functions. So you can just call them, no need to use "emit runTea" \
or "emit reapintTrayIcon" just call them. And for those two functions you don't \
actually need to declare them as slots. (Because you aren't connecting to them \
anyway.) emit is just syntactic sugar for humans, but what emit means is that the \
code is emitting a signal. So use emit only with signals. 

The Teamodel and Settings ctor should take a *const* reference. instead of just a \
reference.

The only other thing I'm not sure about is that position restore code you hace. I'm \
not sure wheter negative postions are really invalid.

daniel
 
> > Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<


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