[Kde-pim] Review Request 119462: Per attendee customization of email [4] - individual mail dialog

[Sandro Knauß]

> On Juli 25, 2014, 1:19 nachm., Kevin Krammer wrote:
> > incidenceeditor-ng/individualmailjobfactory.h, line 72
> > <https://git.reviewboard.kde.org/r/119462/diff/1/?file=292688#file292688line72>
> >
> >     or maybe not have defaults at the method declarations at all, just have them were these methods are called?
> 
> Sandro Knauß wrote:
>     The base class has the exact same defaults, so the derived class should have the same. (And kdepimlibs has *.h in the Messages.sh already)
> 
> Kevin Krammer wrote:
>     Maybe a misunderstanding on my part, but this is not being called by appplication code, is it? It is being called on a pointer to the base class, right?

Yes that is right is is called via a pointer to the base class. Yes you are right, that the defaults are not needed. But isn't it easier to follow the code, if we have the same defaults in client classes?


> On Juli 25, 2014, 1:19 nachm., Kevin Krammer wrote:
> > incidenceeditor-ng/individualmailjobfactory.cpp, line 50
> > <https://git.reviewboard.kde.org/r/119462/diff/1/?file=292689#file292689line50>
> >
> >     it looks like you only needs those two variables for checking "is an email in this list".
> >     So maybe the better type would be QSet<QString>?
> 
> Sandro Knauß wrote:
>     Well actually the list won't be that long and the code doesn't look that nice:
>     
>     QSet<QString> attendeesTo(QSet<QString>::fromList(addressAttribute().to()));
> 
> Kevin Krammer wrote:
>     If you don't like the template syntax use a typedef. The set also makes the check itself easier to read since it explicitly says "contains".
>     There is also no point in artifically pessimizing the code.

List also has the "contains" method. I agree, that contains is the better method to use.


- Sandro


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


On Juli 25, 2014, 9:07 nachm., Sandro Knauß wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119462/
> -----------------------------------------------------------
> 
> (Updated Juli 25, 2014, 9:07 nachm.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Repository: kdepim
> 
> 
> Description
> -------
> 
> The last patch of this serie.
> Crating the actual individual mail dialog, to select which attendee should get an update and for whom a composer should be opened.
> 
> 
> Diffs
> -----
> 
>   incidenceeditor-ng/individualmailjobfactory.cpp PRE-CREATION 
>   incidenceeditor-ng/opencomposerjob.h PRE-CREATION 
>   incidenceeditor-ng/opencomposerjob.cpp PRE-CREATION 
>   incidenceeditor-ng/tests/CMakeLists.txt f62466321239da79b1a56449b73ed16afe211a8c 
>   incidenceeditor-ng/tests/testindividualmaildialog.cpp PRE-CREATION 
>   korganizer/calendarview.cpp df3d74139c2363857adecee308b8a0144183f52d 
>   incidenceeditor-ng/individualmaildialog.h PRE-CREATION 
>   incidenceeditor-ng/individualmaildialog.cpp PRE-CREATION 
>   incidenceeditor-ng/individualmailjobfactory.h PRE-CREATION 
>   incidenceeditor-ng/CMakeLists.txt 9097780defc9922424b36cdb7f576a806c8f1fd4 
>   incidenceeditor-ng/Messages.sh b20d68d1e5b615065b5642b1a0bd419b257543a6 
>   incidenceeditor-ng/editoritemmanager.cpp 7cc3614f6343ead48e899c8ff95caaa2a56dd530 
> 
> Diff: https://git.reviewboard.kde.org/r/119462/diff/
> 
> 
> Testing
> -------
> 
> Sending much of invatations around :)
> 
> 
> File Attachments
> ----------------
> 
> The new dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/07/25/ac439091-58fa-4d8d-a365-810126ff963b__individual-mail.png
> 
> 
> 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/