D7775: Initial submission of Akonadi EWS Resource to KDE PIM

Laurent Montel noreply at phabricator.kde.org
Tue Sep 12 06:05:17 BST 2017


mlaurent added inline comments.

INLINE COMMENTS

> ewsabchpersonhandler.h:33
> +                                                      const Akonadi::Collection &collection) Q_DECL_OVERRIDE;
> +    virtual void setSeenFlag(Akonadi::Item &item, bool value) Q_DECL_OVERRIDE;
> +    virtual QString mimeType() Q_DECL_OVERRIDE;

could you rename all Q_DECL_OVERRIDE to override please ?

> ewscreateabchpersonjob.cpp:44
> +
> +    qCWarning(EWSRES_LOG) << QStringLiteral("Sending abch person items is not supported!");
> +    return false;

could you add EWSRES_LOG to kdepim-runtime.categories please ? so we can load this info in kdebugsettings thanks

> ewsfetchcalendardetailjob.cpp:186
> +        setError(1);
> +        setErrorText(QStringLiteral("Job is not an instance of EwsGetItemRequest"));
> +        emitResult();

i18n ? setErrorText is displaying in a messagebox no ?

> configdialog.cpp:53
> +    : QDialog(), mParentResource(parentResource), mAutoDiscoveryNeeded(false), mTryConnectNeeded(false),
> +      mProgressDialog(Q_NULLPTR)
> +{

could you replace Q_NULLPTR by nullptr thanks :)

> ewscreatecontactjob.h:29
> +public:
> +    EwsCreateContactJob(EwsClient& client, const Akonadi::Item &item,
> +                        const Akonadi::Collection &collection, EwsTagStore *tagStore, EwsResource *parent);

coding style EwsClient &client

> ewsautodiscoveryjob.cpp:93
> +    if (!req) {
> +        setErrorMsg(QStringLiteral("Invalid EwsPoxAutodiscoverRequest job object"));
> +        emitResult();

i18n ?

> CMakeLists.txt:61
> +  ewsxml.cpp
> +  ewsclient_debug.cpp)
> +

debug file is not autogenerated ? see ecm_qt_declare_logging_category(kolabresource_SRCS HEADER kolabresource_debug.h IDENTIFIER KOLABRESOURCE_LOG CATEGORY_NAME org.kde.pim.kolabresource)

> ewsattachment.h:49
> +    ~EwsAttachment();
> +    explicit EwsAttachment(QXmlStreamReader &reader);
> +    EwsAttachment(const EwsAttachment &other);

const here ?

> ewsattendee.h:39
> +    EwsAttendee();
> +    explicit EwsAttendee(QXmlStreamReader &reader);
> +    EwsAttendee(const EwsAttendee &other);

const QXmlStreamReader

> ewsclient.h:34
> +public:
> +    explicit EwsClient(QObject *parent = 0);
> +    ~EwsClient();

replace = 0 to nullptr ?

> ewsclient.h:37
> +
> +    void setUrl(QString url)
> +    {

const QString &

> ewsclient.h:42
> +
> +    void setCredentials(QString username, QString password)
> +    {

const/ref

> ewscreatefolderrequest.cpp:101
> +        if (reader.namespaceUri() != ewsMsgNsUri && reader.namespaceUri() != ewsTypeNsUri) {
> +            setErrorMsg(QStringLiteral("Unexpected namespace in %1 element: %2")
> +                .arg(QStringLiteral("ResponseMessage")).arg(reader.namespaceUri().toString()));

i18n ?

> ewscreateitemrequest.h:42
> +    protected:
> +        Response(QXmlStreamReader &reader);
> +

const QXmk... &

> ewseventrequestbase.cpp:204
> +
> +    if (reader.name() == QStringLiteral("CopiedEvent")) {
> +        mType = EwsCopiedEvent;

why not using evName for each if(...) ?

> ewsitem.cpp:426
> +    // Check what item type are we
> +    if (reader.name() == QStringLiteral("Item")) {
> +        d->mType = EwsItemTypeItem;

cache reader.name() value

> ewsjob.cpp:42
> +
> +bool EwsJob::setErrorMsg(const QString msg, int code)
> +{

const QString &

> ewssubscriberequest.h:63
> +    void setAllFolders(bool allFolders) { mAllFolders = allFolders; };
> +    void setEventTypes(QList<EwsEventType> types) { mEventTypes = types; };
> +    void setWatermark(const QString &watermark) { mWatermark = watermark; };

const ... &

> ewsmtaresource.cpp:38
> +{
> +    qDebug() << "EwsMtaResource";
> +}

Remove debug here no ?

> ewsmailhandler.cpp:82
> +    msg->parse();
> +    qDebug() << msg->attachments().size() << "attachments";
> +    // Some messages might just be empty (just headers). This results in the body being empty.

qCDebug

> basictest.cpp:38
> +public:
> +    BasicTest(QObject *parent = 0);
> +    virtual ~BasicTest();

nullptr + explicit

> fakeewsserverthread.h:32
> +public:
> +    explicit FakeEwsServerThread(QObject *parent = 0);
> +    virtual ~FakeEwsServerThread();

nullptr

> isolatedtestbase.h:65
> +
> +    explicit IsolatedTestBase(QObject *parent = 0);
> +    virtual ~IsolatedTestBase();

nullptr

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/20170912/6daab45e/attachment.html>


More information about the kde-pim mailing list