[Kde-pim] Review Request 110648: Gid support for kdepimlibs

[Volker Krause]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110648/#review36024
-----------------------------------------------------------



akonadi/entity.h
<http://git.reviewboard.kde.org/r/110648/#comment26656>

    These two probably should be moved to Akonadi::Item, since GIDs don't exist for Collections.



akonadi/gid/gidextractor.h
<http://git.reviewboard.kde.org/r/110648/#comment26675>

    should probably be named _p.h, as it's not installed/internal?



akonadi/gid/gidmigrationjob.cpp
<http://git.reviewboard.kde.org/r/110648/#comment26676>

    Shouldn't the migration job rather move to kdepim-runtime, its single place of use?



akonadi/gid/gidmigrationjob.cpp
<http://git.reviewboard.kde.org/r/110648/#comment26677>

    For email you probably want to limit this to Envelope only, not sure about the other types.



akonadi/gid/gidmigrationjob.cpp
<http://git.reviewboard.kde.org/r/110648/#comment26678>

    the base class does emitResult() in case of a subjob error, so we probably need to check for that to avoid double result signals



akonadi/itemcreatejob.cpp
<http://git.reviewboard.kde.org/r/110648/#comment26657>

    I think we only want to do the extraction from the payload if the item doesn't have a GID set already. A resource might have other (and faster) means to retrieve it, than parsing the payload.
    
    Also, this should only be done if there is actually a payload set.



akonadi/itemmodifyjob.cpp
<http://git.reviewboard.kde.org/r/110648/#comment26658>

    This also should consider already set GIDs on the item, independent of the extraction from the payload. And also here the extraction should only happen if there actually is a payload.
    
    We also probably should not look into the payload if the ignorePayload option is set.



akonadi/itemserializer_p.h
<http://git.reviewboard.kde.org/r/110648/#comment26660>

    "default" missing somewhere I think



akonadi/itemserializer_p.h
<http://git.reviewboard.kde.org/r/110648/#comment26659>

    s/with/will/



akonadi/protocolhelper.cpp
<http://git.reviewboard.kde.org/r/110648/#comment26661>

    I don't think GID should be requested by default (consider for example the memory overhead in the KMail message list if we load a few 10k strings with the Message-Id additionally), instead it should be an off-by-default option in ItemFetchScope.



akonadi/typepluginloader_p.h
<http://git.reviewboard.kde.org/r/110648/#comment26674>

    copy/paste typos from above


- Volker Krause


On May 30, 2013, 1:40 p.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110648/
> -----------------------------------------------------------
> 
> (Updated May 30, 2013, 1:40 p.m.)
> 
> 
> Review request for KDEPIM and Volker Krause.
> 
> 
> Description
> -------
> 
> Added support for GID.
> 
> This patch adds support for a globally unique identifier, which is extracted from the payload using the GidExtractorInterface, allowing to fetch items based on the GID. The GID is typically a globally unique UID, such as the UID fields in ical and vcard.
> 
> For testing I added the possibility to inject a testplugin to override the plugin lookup.
> 
> The migrationcode is still missing: I'll add a GidMigrationJob which is run during the servermanager start, which would simply fetch all items of a given mimetype (where we have gid support), and set the gid for each item. From then on gid based lookups should work reliably, allowing for efficient fetching of items which are referred by gid (contactgroups, ical relatedTo).
> 
> 
> Diffs
> -----
> 
>   akonadi/CMakeLists.txt 04a206bed4a9fc08f798ce6116d2eed3f1b13360 
>   akonadi/entity.h a7fb808be2434450f836bd74109627c431da6934 
>   akonadi/entity.cpp fb76859ccde47971651d56ea2ea6cfd6aad84e41 
>   akonadi/entity_p.h 9ed012c5a4433a94b36846bf7f95d93931aae4c7 
>   akonadi/gid/gidextractor.h PRE-CREATION 
>   akonadi/gid/gidextractor.cpp PRE-CREATION 
>   akonadi/gid/gidextractorinterface.h PRE-CREATION 
>   akonadi/gid/gidmigrationjob.cpp PRE-CREATION 
>   akonadi/gid/gidmigrationjob_p.h PRE-CREATION 
>   akonadi/itemcreatejob.cpp 9d3924848ec3f70c1d328576451319241e64b678 
>   akonadi/itemmodifyjob.cpp d81b443dbac0aa96f9092cc7a3700c61aebc4db2 
>   akonadi/itemmodifyjob_p.h 43ec38c2f349859d78158dce8a3499eada25c073 
>   akonadi/itemserializer.cpp be5c263919915716567db713b9b9de60ab5a64dc 
>   akonadi/itemserializer_p.h 5d3f0a67f17c4b9f39b803a5bd335117697f6430 
>   akonadi/protocolhelper.cpp 8e5c088af13c5d9924cf4926349cf8ff4d1064f4 
>   akonadi/protocolhelper_p.h 4f8f3bb5edc493fb36b2609cc9a8fdcbd15e8e38 
>   akonadi/tests/CMakeLists.txt 6ac1538da358b33b752ae8511516bf0f684c04ea 
>   akonadi/tests/gidtest.h PRE-CREATION 
>   akonadi/tests/gidtest.cpp PRE-CREATION 
>   akonadi/typepluginloader.cpp 47683c8144696215e71d4b4628f6a8192622aec8 
>   akonadi/typepluginloader_p.h 659638ca7fdc802ded56f1a7b3e7f129913cc46c 
> 
> Diff: http://git.reviewboard.kde.org/r/110648/diff/
> 
> 
> Testing
> -------
> 
> manual testing, gidtest.
> 
> 
> Thanks,
> 
> Christian Mollekopf
> 
>

_______________________________________________
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/