[Kde-pim] Re: Review Request: Always generate a Message-ID header when composing a message
Ingo Klöcker
kloecker at kde.org
Mon Feb 21 19:39:36 GMT 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100690/#review1565
-----------------------------------------------------------
One more question: Does QHostInfo::localHostName() return the full qualified host name (i.e. host name + domain name)? If not, then we should use QHostInfo::localHostName() + '.' + QHostInfo::localDomainName() as suffix.
messagecomposer/skeletonmessagejob.cpp
<http://git.reviewboard.kde.org/r/100690/#comment1351>
Hmm. Now the ?: operator makes things less readable. :-)
I would simplify this as follows (pseudo code):
QByteArray fqdn;
if ( use custom )
fqdn = custom
if fqdn is empty
fqdn = local host name (+ domain name)
if fqdn is empty
fqdn = 'local.domain'
messagecomposer/skeletonmessagejob.cpp
<http://git.reviewboard.kde.org/r/100690/#comment1349>
Wrong indentation.
Also, I think this error message is superfluous. Almost no user will see it anyway. The config dialog should check that it's not empty. Here we can silently fallback to the host name.
messagecomposer/skeletonmessagejob.cpp
<http://git.reviewboard.kde.org/r/100690/#comment1350>
I'd change this to a kWarning().
- Ingo
On Feb. 21, 2011, 6:40 p.m., Torgny Nyblom wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100690/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2011, 6:40 p.m.)
>
>
> Review request for KDEPIM.
>
>
> Summary
> -------
>
> Always generate a Message-ID header when composing a message. This header is marked as optional according to the RFC but at the same time a comment says that it really SHOULD be present...
>
> There is a catch... the old unit tests will fail since they do not include the Message-ID header so the tests needs to be adapted but I couldn't figure out how (at least not yet).
>
>
> This addresses bug 266063.
> http://bugs.kde.org/show_bug.cgi?id=266063
>
>
> Diffs
> -----
>
> messagecomposer/composer.cpp da4c791
> messagecomposer/skeletonmessagejob.cpp 69b8c3e
> messagecomposer/tests/skeletonmessagejobtest.h d9db557
> messagecomposer/tests/skeletonmessagejobtest.cpp 46c0dca
>
> Diff: http://git.reviewboard.kde.org/r/100690/diff
>
>
> Testing
> -------
>
> Mail generating and sending works.
> New unit test
>
>
> Thanks,
>
> Torgny
>
>
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/
More information about the kde-pim
mailing list