Review Request: Don't use an entire model to loop through contacts

Wed Oct 3 12:36:16 UTC 2012

> On Oct. 2, 2012, 10:07 p.m., Dan Vratil wrote:
> > Nice trick with the loop! 
> > 
> > Just use Q_FOREACH instead of foreach and ship it!
> 
> Aleix Pol Gonzalez wrote:
>     FWIW I don't see the point of considering this an error. If it compiles, it works, if it doesn't then the dependencies are deeply messed up.
> 
> Dan Vratil wrote:
>     If you have Qt compiled with -DQT_NO_KEYWORDS then foreach is not defined, only Q_FOREACH.
> 
> Aleix Pol Gonzalez wrote:
>     You can compile it with QT_NO_CLIPBOARD as well, and not for this reason we're ifdef'ing everything that uses the clipboard. (same with QT_NO_STL, QT_NO_THREAD, QT_NO_INPUTDIALOG, QT_NO_CURSOR, QT_NO_QWS_MOUSE, QT_NO_TEXTEDIT, QT_NO_SYNTAXHIGHLIGHTER and a big etc.)
> 
> David Edmundson wrote:
>     IIRC ktp-call-ui /needs/ to be compiled like that, as otherwise it can clash with keywords in gstreamer, at which point it became a standard "KTp coding style" thing.
>     
>     However, if that is the case then the contact-runner should have this flag set.
>     
>     Anyway, I suggest taking this to the ML if you want to discuss it. (George K or DrDanz will be able to answer better). 
>

As far as I know, the reason why QT_NO_KEYWORDS was introduced is to avoid clash with boost library (I don't know if there is some clash in gstreamer as well), and it's not to be used when compiling Qt, but when you can compile programs that use Qt (just add "add_definitions (-DQT_NO_KEYWORDS)" in CMakeFile.txt to try it) (but perhaps you can compile Qt as well with that? I don't know)

In my opinion QT_NO_KEYWORDS is mandatory when:
1) you are writing and executable and you are using both Qt and boost libraries.
2) you are writing a Qt library (someone else might use it together with boost).

If you are writing an executable and you don't use boost (or any other clashing library), it should not be a problem.
I don't see any good reason not to use it, so I usually use Q_* macros everywhere for coherence, and because I like it more than the other version.
Anyway it's up to you whether to use it or not, but since the maintainer is Dan, I'd say fix it ;)

(By the way, if I remember correctly, we had a discussion about this with Dario at one of the sprints and the conclusion was to define -DQT_NO_KEYWORDS and -DQT_NO_CAST_FROM_ASCII in every KTp component)

- Daniele Elmo

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

On Oct. 2, 2012, 9:45 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106700/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2012, 9:45 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Don't use an entire model to loop through contacts.
> 
> Should be faster, and simpler.
> 
> 
> Diffs
> -----
> 
>   src/contactrunner.h 55e7e9de017dca23fa1d6551d4ce99997d05cad3 
>   src/contactrunner.cpp 48e01bf10b902932631b6e987c191d4d868e7d82 
> 
> Diff: http://git.reviewboard.kde.org/r/106700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20121003/d3860c46/attachment.html>