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

List:       kopete-devel
Subject:    Re: [Kopete-devel] [Patch] Fix for bug #68416
From:       Martijn Klingens <klingens () kde ! org>
Date:       2003-11-30 20:54:34
[Download RAW message or body]

On Sunday 30 November 2003 21:47, Till Gerken wrote:
> +void JabberEditAccountWidget::toggleJIDValidation ()
> +{
> +       revalidateJID = true;
> +}

Nitpicking: either it should read 'revalidateJID = !revalidateJID' or the 
method should be renamed to setJIDValidation, because this isn't called 
toggling :)

One question: is it possible to trigger a case when no validation is needed? 
I'd say when the text is restored to the original value, but that requires so 
much code to get right it's no fun and I'm not asking you for that (there's 
stuff like the new config compiler to handle that IIRC, never tried it). 
Can't think of another case.

Next, a request: add '-p' to your cvs diff line in .cvsrc, so it also mentions 
the method name. Makes diffs slightly easier to read.

Lastly, with the above nitpick corrected or not, the code is ok for commit 
anyway. Looks fine :)

-- 
Martijn
_______________________________________________
Kopete-devel mailing list
Kopete-devel@kde.org
https://mail.kde.org/mailman/listinfo/kopete-devel

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