[Kde-pim] Review Request 119434: Per attendee customization of email [2] - DialogInterception for IncienceChanger

[Kevin Krammer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119434/#review63036
-----------------------------------------------------------



akonadi/calendar/incidencechanger.cpp
<https://git.reviewboard.kde.org/r/119434/#comment43752>

    what is the reason for the queued connection?



akonadi/calendar/incidencechanger.cpp
<https://git.reviewboard.kde.org/r/119434/#comment43755>

    preferably no const for int arguments



akonadi/calendar/incidencechanger.cpp
<https://git.reviewboard.kde.org/r/119434/#comment43750>

    usually no const for arguments of primitive types



akonadi/calendar/incidencechanger.cpp
<https://git.reviewboard.kde.org/r/119434/#comment43751>

    maybe use QString() instead? or does this need to be an empty, non-null string?



akonadi/calendar/incidencechanger.cpp
<https://git.reviewboard.kde.org/r/119434/#comment43753>

    QString()?



akonadi/calendar/incidencechanger.cpp
<https://git.reviewboard.kde.org/r/119434/#comment43754>

    that comment looks like a temporary code piece. remove?



akonadi/calendar/incidencechanger_p.h
<https://git.reviewboard.kde.org/r/119434/#comment43756>

    style nitpick: space before :



akonadi/calendar/incidencechanger_p.h
<https://git.reviewboard.kde.org/r/119434/#comment43757>

    well, it is a _p.h, we sometimes do have small inline code in those



akonadi/calendar/itiphandler.h
<https://git.reviewboard.kde.org/r/119434/#comment43758>

    space before :



akonadi/calendar/itiphandler.h
<https://git.reviewboard.kde.org/r/119434/#comment43759>

    no explicit needed, there are three required arguments.
    parent should probably be last, with 0 as default?



akonadi/calendar/itiphandler.h
<https://git.reviewboard.kde.org/r/119434/#comment43762>

    maybe move to end to allow for default value?
    might also be worth considering an enum, makes the calls easier to read, e.g.
    
    askUserIfNeeded("some question", "yes", "no", AskDelegator::IgnoreDefaultAction)



akonadi/calendar/itiphandler.h
<https://git.reviewboard.kde.org/r/119434/#comment43760>

    just as a general remark: it is helpful for translators to have some context when the texts are short.
    
    ki18nc("@action:button dialog positive answer", "Send Email")



akonadi/calendar/itiphandler.h
<https://git.reviewboard.kde.org/r/119434/#comment43761>

    this is a new class, right? I.e. it has not been part of a release yet?


- Kevin Krammer


On Juli 23, 2014, 8:50 nachm., Sandro Knauß wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119434/
> -----------------------------------------------------------
> 
> (Updated Juli 23, 2014, 8:50 nachm.)
> 
> 
> Review request for KDEPIM-Libraries.
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> To be able to customize the emails per attendee from an iTip event. The dialogs opened by IncidenceChange has to be interceptable. You can see the both changeset in the dev/individual_mail2 branch.
> 
> 
> Diffs
> -----
> 
>   akonadi/calendar/incidencechanger_p.h d1c0e80531d8daef5c393c10fd9d5e322cb88293 
>   akonadi/calendar/itiphandler.h 56aaedaca2a33b74f2206976313d645d7b50d4b2 
>   akonadi/calendar/itiphandler.cpp 03a3adb9e4201c835b7be7ceade4c57285df13a8 
>   akonadi/calendar/itiphandler_p.h a0bb98e1380fc91c4bab3931b88a0f8212007f9c 
>   akonadi/calendar/itiphandler_p.cpp 4d024bc597ce77882424e522f06b46e376e1c3d5 
>   akonadi/calendar/itiphandlerhelper_p.h 466bd8c0010dcdf1e8e03469e99942e10af50ac4 
>   akonadi/calendar/itiphandlerhelper_p.cpp 1b6d03f03f54c955f8a704e9121d7745b1cfbf8e 
>   akonadi/calendar/tests/CMakeLists.txt 382e3ec81e189085e4fbf9f34a3be9e6511e52f7 
>   akonadi/calendar/incidencechanger.cpp 0059f2aa37c7af0e0101b2a0540e64aaf8d83258 
> 
> Diff: https://git.reviewboard.kde.org/r/119434/diff/
> 
> 
> Testing
> -------
> 
> run multiple times kincidenceeditor and korganizer to test and run itiphelpertest and mailclienttest.
> 
> 
> Thanks,
> 
> Sandro Knauß
> 
>

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