<table><tr><td style="">kossebau added inline comments.
</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/D3646" rel="noreferrer">View Revision</a></tr></table><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/D3646#inline-14772" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Android.cmake:163</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Maybe call it <tt style="background: #ebebeb; font-size: 13px;">ECM_ALTERNATIVE_ROOT_PATH</tt>, in case we ever need to use it on other platforms as well?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Hm, indeed a point in that ECM_ANDROID_ROOT_PATH sounds like this is the only one and might replace the root set by the ndk. So would agree a better term should be found.</p>
<p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">ALTERNATIVE</tt> sounds a little bit strange to me initially, also not part of any existing terms in cmake/ecm vars for what I found. But given it is used in the description of CMAKE_FIND_ROOT_PATH (see <a href="https://cmake.org/cmake/help/v3.1/variable/CMAKE_FIND_ROOT_PATH.html?highlight=alternative" class="remarkup-link" target="_blank" rel="noreferrer">https://cmake.org/cmake/help/v3.1/variable/CMAKE_FIND_ROOT_PATH.html?highlight=alternative</a>) for the values passed here, and in the end is pretty self explaining.<br />
Or something like <tt style="background: #ebebeb; font-size: 13px;">ECM_ADDITIONAL_FIND_ROOT_PATH</tt> to pick up the cmake-naming and still point out this are just additional ones? Perhaps with a better term than <tt style="background: #ebebeb; font-size: 13px;">ADDITIONAL</tt>?</p></div></div><br /><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/D3646#inline-14771" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Android.cmake:188</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This is unrelated. Also maybe while at it you can document it right away.<br />
I just added it so that we can detect problems at config time rather than while linking.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">So gnustl_shared is required by Qt for Android? Any link for backing this up which I could use to please curious people? I could not find anything when I quickly searched, so this here seemed just some magic checking :)</p></div></div><br /><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/D3646#inline-14770" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Android.cmake:209</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">That's not requried, as the NDK already passes the <tt style="background: #ebebeb; font-size: 13px;">__ANDROID__</tt>.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This was just moved around (see l.167 before), as part of grouping things together instead of interleaved setup (as also mentioed at end of summary text). Am to be blamed for trying too many things in one review, even if related, will see to keep things as simple as possible in the future.</p>
<p style="padding: 0; margin: 8px;">This was added in <a href="https://phabricator.kde.org/R240:faedc8d01697ab1d573d5740f24e7279f4dba14f" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">faedc8d01697ab1d573d5740f24e7279f4dba14f</a>. <br />
So it was now found this would not be needed? Which Qt versions would <tt style="background: #ebebeb; font-size: 13px;">__ANDROID__</tt> work with? <a href="https://phabricator.kde.org/p/cordlandwehr/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@cordlandwehr</a> , your commit to be defended here, please tell :)</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>avoidHostLibsIncludesinAndroidToolchain</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3646" rel="noreferrer">https://phabricator.kde.org/D3646</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>kossebau, Frameworks, GCompris, Minuet, apol, mutlaqja, sandsmark, nienhueser, cordlandwehr<br /></div>