<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/106700/">http://git.reviewboard.kde.org/r/106700/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 2nd, 2012, 10:07 p.m., <b>Dan Vratil</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Nice trick with the loop! 

Just use Q_FOREACH instead of foreach and ship it!</pre>
 </blockquote>




 <p>On October 2nd, 2012, 10:11 p.m., <b>Aleix Pol Gonzalez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
 </blockquote>





 <p>On October 2nd, 2012, 10:21 p.m., <b>Dan Vratil</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If you have Qt compiled with -DQT_NO_KEYWORDS then foreach is not defined, only Q_FOREACH.</pre>
 </blockquote>





 <p>On October 2nd, 2012, 10:44 p.m., <b>Aleix Pol Gonzalez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.)</pre>
 </blockquote>





 <p>On October 2nd, 2012, 10:47 p.m., <b>David Edmundson</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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). 
</pre>
 </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)
</pre>
<br />








<p>- Daniele Elmo</p>


<br />
<p>On October 2nd, 2012, 9:45 p.m., David Edmundson wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Telepathy.</div>
<div>By David Edmundson.</div>


<p style="color: grey;"><i>Updated Oct. 2, 2012, 9:45 p.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Don't use an entire model to loop through contacts.

Should be faster, and simpler.</pre>
  </td>
 </tr>
</table>





<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>src/contactrunner.h <span style="color: grey">(55e7e9de017dca23fa1d6551d4ce99997d05cad3)</span></li>

 <li>src/contactrunner.cpp <span style="color: grey">(48e01bf10b902932631b6e987c191d4d868e7d82)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/106700/diff/" style="margin-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>