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





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 7th, 2014, 8:46 p.m. CET, <b>Commit Hook</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;">This review has been submitted with commit 676e746b78d8b6ada47dd15fd706d0deb9996e45 by Alex Merry to branch master.</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;">A bit late, but how about adding the include(GenerateExportHeader) to KDECMakeSettings.cmake, this way the frameworks don't have to be updated.
I guess most users of KDECMakeSettings will need GenerateExportHeader and even if it is uneeded I doubt it has any noticeable impact on the configure time.

I think it would be nice in order to reduce the amount of CMake boilerplate for KDE applications/libraries.

</pre>
<br />










<p>- Alexander</p>


<br />
<p>On January 7th, 2014, 8:46 p.m. CET, Alex Merry wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Build System, KDE Frameworks and Stephen Kelly.</div>
<div>By Alex Merry.</div>


<p style="color: grey;"><i>Updated Jan. 7, 2014, 8:46 p.m.</i></p>









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


<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;">Remove use of obsolete add_compiler_export_flags

Set CMAKE_CXX_VISIBILITY_PRESET and CMAKE_VISIBILITY_INLINES instead
(which sets the default for all targets).

Note that the removal of include(GenerateExportHeader) means that this
will have to be explicitly included in the CMakeLists.txt of the
frameworks (as they use generate_export_header).</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;">Threadweaver builds, once include(GenerateExportHeader) is added to its CMakeLists.txt, and properly passes -fvisibility=hidden -fvisibility-inlines-hidden to the library code (although not to the test code).</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>kde-modules/KDECompilerSettings.cmake <span style="color: grey">(72824e166d03dcc2d089814dc121f08ba998974a)</span></li>

</ul>

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







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








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