D7775: Initial submission of Akonadi EWS Resource to KDE PIM

[Laurent Montel]

mlaurent added inline comments.

INLINE COMMENTS

> CMakeLists.txt:31
> +
> +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config.h)
> +

could you rename it as config-ews.h.cmake ... config-ews.h ?
It avoid to have a lot file with same name.
Thanks

> ewsabchpersonhandler.h:42
> +    static EwsItemHandler *factory();
> +private:
> +};

not necessary

> ewscreateabchpersonjob.h:34
> +protected:
> +    virtual void doStart() override;
> +};

remove virtual keyword

> ewsfetchabchpersondetailjob.h:32
> +protected:
> +    virtual void processItems(const QList<EwsGetItemRequest::Response> &responses) override;
> +};

Remove all virtual keyword when you have override thanks

> configdialog.cpp:59
> +
> +    mButtonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
> +    QWidget *mainWidget = new QWidget(this);

Add parent to QDialogButtonBox it can create a bug where default button is not correct (for example it can select cancel and not ok in some case)
thanks

> ewsautodiscoveryjob.h:40
> +        return mEwsUrl;
> +    };
> +    const QString &oabUrl() const

pedantic no .? we don't need ";"
Same for all other methods

> ewscreatefolderrequest.h:43
> +            return mId;
> +        };
> +    protected:

pedantic

> ewsjob.h:33
> +    virtual bool doKill() override;
> +    bool setErrorMsg(const QString msg, int code = KJob::UserDefinedError);
> +};

const QString &msg

> ewsrecurrence.cpp:84
> +
> +        if (reader.name() == QStringLiteral("RelativeYearlyRecurrence")) {
> +            if (!readRelativeYearlyRecurrence(reader)) {

cache reader.name() seems better

> ewsupdateitemrequest.cpp:214
> +
> +    Q_FOREACH (const QSharedPointer<const Update> upd, mUpdates) {
> +        if (!upd->write(writer, mType)) {

const QSharedPointer &upd

> ewsmtaresource.h:48
> +    void retrieveItems(const Akonadi::Collection &collection) override;
> +#if (AKONADI_VERSION > 0x50328)
> +    bool retrieveItems(const Akonadi::Item::List &items, const QSet<QByteArray> &parts) override;

I think that you remove check

REPOSITORY
  R44 KDE PIM Runtime

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

To: nowicki, #kde_pim, dvratil, mlaurent
Cc: kde-pim, dvasin, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20171002/01fe6596/attachment.html>