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

List:       kopete-devel
Subject:    Re: [Kopete-devel] kopetecontact again
From:       Martijn Klingens <klingens () kde ! org>
Date:       2003-07-11 18:08:41
[Download RAW message or body]

On Thursday 10 July 2003 23:41, Grzegorz Jaskiewicz wrote:
> > > Of course my plan was to remove this field from kopeteonlinestatus - if
> > > so, i will leave it as statusDescription).
> >
> > Hmm, I added it for future use. I'd have to review it, but AFAIK you
> > can't remove it. Please wait with that for now. If we do it at all,
> > please make it a separate commit.
>
> Well, our enemy is anything that is "reserved for future usage" or
> simmilar. Becouse it does the same thing at the moment, and/or is
> redundant if you want to keep my changes - i will remove it straight away.

But it's not obsolete at all... It's the description for all online statuses. 
It should be used in e.g. the "What's This?" help for the 'Set xxxx Status' 
actions.

Your code doesn't cover that (nor should it), it serves an entirely different 
goal.

> Another thing, can somebody tell me where chat windows caption is
> generated ? I guess we will have to use this property instead of
> kopeteonlinestatus one.

Why use that caption and not KOS ?

Now, the patch:

> -void KopeteContact::setOnlineStatus( const KopeteOnlineStatus &status )
> +void KopeteContact::setOnlineStatus( const KopeteOnlineStatus &status,
> +                                       const QString &statusDescription)
>  {
> -  if( status == d->onlineStatus)
> +  if(status == d->onlineStatus && statusDescription==d->statusDescription)
>      return;

<nitpicking>Please don't change the coding style, you're removing spaces 
around parentheses here.

> -
> +       

<more-nitpicking>Please be careful not to add trailing whitespace. You add 
extra tabs (or spaces?) at the end of a line.

> +        * Set the contact's online status and status reason
> +        * By default description is set to QString::null,
> +        * and if you will not change it - other parts
> +        * of kopete will assume that you are not using this property.

What do you mean with this? The description is not entirely clear to me 
(notably the "other parts of kopete will assume that you are not using this 
property" part). Perhaps you need to change it a bit ;)

> +       */
> +       void setOnlineStatus( const KopeteOnlineStatus &status,
> +                               const QString &statusDescription);

Where did my proposed default value for statusDescription (' = QString::null') 
go? IMO the new parameter should be optional, not mandatory.

> +       /**
> +        * Returns QString::null if protocol do not use it,
> +        * or status description otherwise. Gives empty QString (not null),
> +        * if current status does not require description.
> +        */

You may want to learn the doxygen syntax and use stuff like @return for Matt's 
online API docs, but I should do that myself too, so I'm not the best person 
to tell you this :)

-- 
Martijn

_______________________________________________
Kopete-devel mailing list
Kopete-devel@mail.kde.org
http://mail.kde.org/mailman/listinfo/kopete-devel
[prev in list] [next in list] [prev in thread] [next in thread]