From kde-pim  Wed Aug 20 14:59:28 2008
From: Togge <togge.gentoo () gmail ! com>
Date: Wed, 20 Aug 2008 14:59:28 +0000
To: kde-pim
Subject: Re: [Kde-pim] [RFC, Patch] KMail account wizard
Message-Id: <200808201659.34485.togge.gentoo () gmail ! com>
X-MARC-Message: https://marc.info/?l=kde-pim&m=121924443425050

Hi,

On Tuesday 19 August 2008 18:57:28 Thomas McGuire wrote:
[...]
> Thanks for the patch. There are some problems which need to be fixed before
> it can be committed:
>
> When invoking the wizard from the tool menu, it says "It seems you have
> started KMail the first time...". Please fix that.
Fixed.
I added a function to kmkernel to be able to recognize when the wizard had 
been used once before, the variable used was only written to at startup 
(KMKernel::init()) so the first page of the wizard would always be shown 
before kmail had been restarted once. This also seems to have fixed the layout 
problem below.

> The layout of the identity chooser page is broken, see attached screenshot.
Fixed, I hope since I couldn't open the screenshot might be my kmail that is 
borked...).

> The identity which created is named "Unnamed". Maybe use the same naming
> scheme as for the auto-created accounts?
Fixed. It now uses a similar approach, ie using the surname as identity name 
instead of the domain part of the email address.

> When re-using an existing identity, it overwrites the special transport
> setting. Please don't change the identity at all when reusing it. In fact,
> I think createIdentity shouldn't do anything when reusing an identity.
Fixed.

> Factor out the "if( manager->identities().count() == 1 && !manager-
> >defaultIdentity().mailingAllowed() )" into a common method to avoid code
> duplication, for example onlyDefaultIdentity() or something like that.
Fixed.

> Also factor out the clear() calls to a separate method.
Fixed.

> Actually, I wonder why this is called at all, isn't the text for those
> fields later overwritten in slotCurrentPageChanged() anyway, or am I
> missing something?
The fields are only written to if they are empty.

> Coding style wrt spaces around parenthesis, for example in the
> setAppropriate() call or at "if( mIdentityButtonGroup...".
> Change to "setAppropriate( mIdentityChooserPage, ..." and
> "if ( mIdentityButtonGroup...".
Fixed (I hope :) )

New patch at: http://www.nyblom.org/pub/kde/kmail-accountwizard-2.patch

Any thing else I missed?

/Regards
Torgny
_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/