<table><tr><td style="">kossebau added a comment.
</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/D29299">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D29299#660453" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D29299#660453</a>, <a href="https://phabricator.kde.org/p/pino/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@pino</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>The problem is that ki18n_install() is rarely used manually, and generally it is appended by the release scripts to the top-level CMakeLists.txt file that goes into the tarballs.</p></div>
</blockquote>
<p>While being an orthogonal topic, IMHO this should be changed. There is just too much magic going on when it comes to translations, the avaerage KDE devloper does not understand it, at least by what I gathered :) Also would some not consider it a good practice to patch sources before adding to the tarball, making the sources different to what is tagged.</p>
<p>Personally I would favour rather explicit statements, with a comment explaining why there is no po/ file in the sources and a link to a website where to learn more about things.</p>
<blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D29299#660465" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D29299#660465</a>, <a href="https://phabricator.kde.org/p/pino/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@pino</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>Also, your patch basically includes <a href="https://phabricator.kde.org/D29136" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: line-through;">D29136</a> in the case of no DESTINATION parameter specified, hence my suggestion is:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">edit <a href="https://phabricator.kde.org/D29136" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: line-through;">D29136</a> to do the fallback using the same logic introduced here: this way marble is already fixed with no other changes, and ki18n_install will work also with KDE_INSTALL_DIRS_NO_DEPRECATED (e.g. for release-service packages)</li>
<li class="remarkup-list-item">have this to add the DESTINATION parameter, so packages can opt-in to use it if they can/want</li>
</ul></div>
</blockquote>
<p>Not exactly sure what you mean? Do you want two separate commits/reviews, one per issue?</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R249 KI18n</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D29299">https://phabricator.kde.org/D29299</a></div></div><br /><div><strong>To: </strong>kossebau, ilic, heikobecker, Frameworks, aacid, ltoscano<br /><strong>Cc: </strong>pino, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns<br /></div>