<table><tr><td style="">heikobecker requested changes to this revision.<br />heikobecker added a comment.<br />This revision now requires changes to proceed.
</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/D24817">View Revision</a></tr></table><br /><div><div><p>How are those bindings generated? I assume they change depending on the used Qt version? Could it be feasible to convince cmake to generate them during build time?</p>

<p>Not only the generated files, but also some others you added are missing a lincese header.</p>

<p>However, I'm still not sure if it's worth to pursue this, QtScript is going away, no way around it. And if it turns out we break scripts, it would be nice to only do it once instead of twice. Ideally, I'd like to hear feedback from people more involved than me.</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/D24817#inline-155665">View Inline</a><span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:1404</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);">                qtscript_xml
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        DESTINATION lib/plugins/script)
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This breaks for people with lib{32,64,whatever} dirs, there's <a href="https://api.kde.org/ecm/kde-module/KDEInstallDirs.html" class="remarkup-link" target="_blank" rel="noreferrer">https://api.kde.org/ecm/kde-module/KDEInstallDirs.html</a> to handle this properly.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R181 Amarok</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D24817">https://phabricator.kde.org/D24817</a></div></div><br /><div><strong>To: </strong>nicolamori, amarok-devel, heikobecker<br /><strong>Cc: </strong>malteveerman, heikobecker, Amarok, amarok-devel<br /></div>