D15065: Implement Unified Mailboxes agent

Laurent Montel noreply at phabricator.kde.org
Sun Sep 9 13:39:33 BST 2018


mlaurent requested changes to this revision.
mlaurent added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> mailkernel.cpp:32
> +
> +MailKernel::MailKernel(KSharedConfigPtr config, QObject *parent)
> +    : QObject(parent)

const'ref

> mailkernel.h:41
> +public:
> +    explicit MailKernel(KSharedConfigPtr config, QObject *parent = nullptr);
> +    ~MailKernel() override;

const'ref here too

> settingsdialog.cpp:42
> +
> +static constexpr const char *DialogGroup = "__Dialog";
> +

it's better to use "UnifedMailBoxAgentSettingDialog" as name so we can know which settings to remove in settings file when we want to reset to default.

> settingsdialog.cpp:52
> +{
> +    auto l = new QVBoxLayout;
> +    setLayout(l);

auto l = new QVBoxLayout(this);

> you can remove the next line setLayout(l)
===========================================

> settingsdialog.h:36
> +public:
> +    explicit SettingsDialog(KSharedConfigPtr config, UnifiedMailboxManager &manager,
> +                            WId windowId, QWidget *parent = nullptr);

const KSharedConfigPtr &

> unifiedmailboxeditor.cpp:42
> +
> +static constexpr const char *EditorGroup = "__Editor";
> +

UnifiedMailboxEditorDialog as name

> unifiedmailboxeditor.cpp:97
> +{
> +    auto l = new QVBoxLayout;
> +    setLayout(l);

auto l = new QVBoxLayout(this);
Remove next line 'setLayout(l);

> unifiedmailboxeditor.cpp:166
> +            });
> +    box->button(QDialogButtonBox::Ok)->setEnabled(!nameEdit->text().isEmpty());
> +    l->addWidget(box);

!nameEdit->text().trimmed().isEmpty(); otherwise we can name a folder as <space> :)

same for setEnabled(...) in previous line

> kmail.categories:7
>  org.kde.pim.mailfilteragent kmail (mailfilter agent)
> -
> +org.kde.pim.unifiedmailboxagent kmail (unifiedmailboxagent)

I think you need to rebase against master as in this file I use new syntax

> add IDENTIFIER [....]
=======================

REPOSITORY
  R206 KMail

REVISION DETAIL
  https://phabricator.kde.org/D15065

To: dvratil, mlaurent
Cc: lueck, ngraham, mlaurent, kde-pim, dvasin, rodsevich, winterz, vkrause, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20180909/76c24e69/attachment.html>


More information about the kde-pim mailing list