<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/110953/">http://git.reviewboard.kde.org/r/110953/</a>
     </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On June 11th, 2013, 4:58 p.m. UTC, <b>David Narváez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/110953/diff/1/?file=149558#file149558line12" style="color: black; font-weight: bold; text-decoration: underline;">KigConfigureChecks.cmake</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">12</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb"><span class="hl">macro_optional_</span>find_package</span><span class="p">(</span><span class="s">BoostPython</span><span class="p">)</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">12</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">find_package</span><span class="p">(</span><span class="s">BoostPython</span><span class="p">)</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">What would you say about moving this and the macro_log_feature call below to CMakeLists.txt? I believe it is more visible there.</pre>
 </blockquote>



 <p>On June 12th, 2013, 3:57 a.m. UTC, <b>Vadim Zhukov</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;">Personally I don't see any point in having KigConfigureChecks.cmake file at all. But this is probably for another patchset, to not complicate all current Kig reviews. Okay to post-pone this task?</pre>
 </blockquote>





 <p>On June 12th, 2013, 5:38 a.m. UTC, <b>David Narváez</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;">I agree, so I think moving the Boost Python stuff outside KigConfigureChecks.cmake takes us a step towards removing that file completely so that's my reason to wonder if we could do that in this patch.</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">As 4.11 branches are now created, I'm retaking this (and the other) patch in order to commit them to master (as we were already pushing one rather large refactor to FindBoostPython.cmake in 4.11 and I didn't want to mix these up after the first betas). After applying it I have two more comments:

- My boost python detection failed for a reason that has nothing to do with this patch, but my configure messages ended like this:

[...]
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.28") 
-- Looking for C++ include boost/shared_ptr.hpp
-- Looking for C++ include boost/shared_ptr.hpp - found

-----------------------------------------------------------------------------
-- The following OPTIONAL packages could NOT be located on your system.
-- Consider installing them to enable more features from this software.
-----------------------------------------------------------------------------
   * Boost.Python (1.31 or higher)  <http://www.boost.org/>
     Kig can optionally use Boost.Python for Python scripting

-----------------------------------------------------------------------------

-- Configuring done
[...]

a regular user would be clueless of what went wrong, can we improve feedback in the sections after finding shared_ptr.hpp?

- Also, you see the message above reads 1.31 or higher, but FindBoostPython.cmake has a line that reads find_package(Boost 1.33 COMPONENTS python), so we need to pick one of the two, the oldest one.</pre>
<br />




<p>- David</p>


<br />
<p>On June 11th, 2013, 3:08 p.m. UTC, Vadim Zhukov wrote:</p>








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

<div>Review request for KDE Edu and David Narváez.</div>
<div>By Vadim Zhukov.</div>


<p style="color: grey;"><i>Updated June 11, 2013, 3:08 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;">Polish FindBoostPython.cmake from CMake's point of view:
  
 * Use BoostPython_ prefix instead of BOOST_PYTHON_
   (BOOSTPYTHON_ is also acceptable but is unreadable);

 * Remove extra call to find_package(BoostPython);

 * Replace macro_optional_find_package() with simple find_package(),
   CMake has CMAKE_DISABLE_FIND_PACKAGE_Foo feature for a while.

This is a first patch of a bigger set, containing only minor fixes.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">OpenBSD-CURRENT, CMake 2.8.11, KDE 4.10.4, Boost 1.53, Python 3.3 and 2.7.</pre>
  </td>
 </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=320807">320807</a>


</div>


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

 <li>CMakeLists.txt <span style="color: grey">(3c3da29)</span></li>

 <li>KigConfigureChecks.cmake <span style="color: grey">(fed38b3)</span></li>

 <li>cmake/modules/FindBoostPython.cmake <span style="color: grey">(d6c5a34)</span></li>

</ul>

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







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








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