<table><tr><td style="">mpyne accepted this revision.<br />mpyne added a comment.<br />This revision is now accepted and ready to land.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D5430" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>I think these warning flags make sense as default warning flags.  I don't agree that <tt style="background: #ebebeb; font-size: 13px;">__DATE__</tt> and <tt style="background: #ebebeb; font-size: 13px;">__TIME__</tt> should be avoided as a rule, but projects that find this valuable can disable the warning as appropriate (e.g. with a "developer mode" option) and make alternative provisions for reproducible builds (for packagers or those auditing the binaries generated by others).</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5430#inline-22333" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">KDECompilerSettings.cmake:376</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">endif</span><span class="p">()</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">if</span> <span class="p">(</span><span style="color: #766510">CMAKE_CXX_COMPILER_ID</span> <span style="color: #766510">STREQUAL</span> <span style="color: #766510">"GNU"</span> <span style="color: #766510">AND</span> <span style="color: #766510">NOT</span> <span style="color: #766510">CMAKE_CXX_COMPILER_VERSION</span> <span style="color: #766510">VERSION_LESS</span> <span style="color: #766510">5.0</span> <span style="color: #766510">OR</span> <span class="p">(</span><span style="color: #766510">CMAKE_CXX_COMPILER_ID</span> <span style="color: #766510">MATCHES</span> <span style="color: #766510">"Clang"</span> <span style="color: #766510">AND</span> <span style="color: #766510">NOT</span> <span style="color: #766510">CMAKE_CXX_COMPILER_VERSION</span> <span style="color: #766510">VERSION_LESS</span> <span style="color: #766510">3.5</span><span class="p">))</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #74777d"># -Wdate-time: warn if we use __DATE__ or __TIME__ (we want to be able to reproduce the exact same binary)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">The CMake docs seem to say that the operator precedence of OR vs AND will do what you mean here for the GCC version check.  But still, if you're going to defensively parenthesize the Clang version check (and I agree with doing so), you should also do the same for the GCC version check.  That way we don't have to check the CMake docs to make sure it's right. :)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R240 Extra CMake Modules</div></div></div><br /><div><strong>BRANCH</strong><div><div>master</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D5430" rel="noreferrer">https://phabricator.kde.org/D5430</a></div></div><br /><div><strong>To: </strong>kfunk, mpyne<br /><strong>Cc: </strong>mpyne, Frameworks, Build System<br /></div>