<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/117078/">https://git.reviewboard.kde.org/r/117078/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<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 few comments, but none of them is a blocker. Well, the last one should be fixed, but you can push after fixing it.
Nice work, thanks.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117078/diff/4/?file=262862#file262862line427" style="color: black; font-weight: bold; text-decoration: underline;">autotests/kfileitemtest.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KFileItemTest::testIconNameForUrl()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">427</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#ifndef Q_OS_WIN </span><span class="c1">// user/group/other write permissions are not handled on windows</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Could be replaced with a Windows-specific test for KFileItem::isReadable, though.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117078/diff/4/?file=262870#file262870line25" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kioglobal_p_win.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">25</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#define _WIN32_WINNT 0x600 </span><span class="c1">//Vista is minimum supported windows version (we need CreateSymbolicLinkW)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So this won't compile on XP at all?</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117078/diff/4/?file=262870#file262870line108" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kioglobal_p_win.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">108</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">qWarning</span><span class="p">(</span><span class="s">"KIOPrivate::changeOwnership: not implemented yet"</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Nice implementation :-)
Quite a corner case though. Even on Unix this is almost never called: it requires running an app as root in the first place.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/117078/diff/4/?file=262873#file262873line777" style="color: black; font-weight: bold; text-decoration: underline;">src/ioslaves/file/file.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString FileProtocol::getGroupName(gid_t gid) const</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString FileProtocol::getGroupName(KGroupId gid) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">765</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">char</span> <span class="n">buffer2</span><span class="p">[</span> <span class="mi">1000</span> <span class="p">];</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">775</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">QString</span> <span class="n">linkTarget</span> <span class="o">=</span> <span class="n">QFile</span><span class="o">::</span><span class="n">symLinkTarget</span><span class="p">(</span><span class="n">QString</span><span class="o">::</span><span class="n">fromLocal8Bit</span><span class="p">(</span><span class="n">path</span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Should be QFile::decodeName() rather than QString::fromLocal8Bit().</pre>
</div>
<br />
<p>- David Faure</p>
<br />
<p>On April 4th, 2014, 7:20 p.m. UTC, Alexander Richardson 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 KDE Frameworks.</div>
<div>By Alexander Richardson.</div>
<p style="color: grey;"><i>Updated April 4, 2014, 7:20 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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;">A series of commits (can submit separately if necessary):
---
Add S_IXUSR, IRUSR, etc. definitions for Windows
---
Add KIOPrivate::isProcessAlive() and KIOPrivate::sendTerminateSignal()
kill() does not work on Windows, therefore a WIN32 implementation is needed
in order to compile slave.cpp
---
Add KIOPrivate::changeOwnership() to avoid directly calling chown()
Additionally use KUserId/KGroupId instead of uid_t/gid_t.
This allows compiling chmodjob.cpp on Windows.
KIOPrivate::changeOwnership is a stub on Windows. However, it has always
been like that with the kdewin chmod() implementation, so this is not a
regression from kdelibs4.
---
Add KIOPrivate::createSymlink() to avoid using symlink() directly
---
Minor Windows compile fixes
---
Use KUser in KPropertiesDialog
This means no more need for getpwent(), etc -> works on Windows
---
Export the KIOPrivate functions
---
Fix kio_http build on Windows
---
No longer use uid_t/gid_t in kio_file
---
Allow compiling kurlcompletion.cpp on windows
---
Add a fake QT_LSTAT for Windows
This just calls QT_STAT and adds QT_STAT_LNK flag to st_mode if it is a
symlink
---
Use KUser in kpropertiesdialog.cpp -> no more getgrouplist
</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;">compiles, tests still the same as before (i.e. not passing)</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>autotests/fileundomanagertest.cpp <span style="color: grey">(3f209f89cc0e2ac48d8eaef7ee73ec18abca9a4c)</span></li>
<li>autotests/globaltest.cpp <span style="color: grey">(4367e53b44e077c566316081e21f429ac15b74a0)</span></li>
<li>autotests/kdirlistertest.cpp <span style="color: grey">(10a970416b8874f1e136b460d05378f8f3a86810)</span></li>
<li>autotests/kfileitemtest.h <span style="color: grey">(dc1808e726cf6af1605cfda520c7df3832225cc1)</span></li>
<li>autotests/kfileitemtest.cpp <span style="color: grey">(38bd87f4e9facd8e52e9e5fbd98b16011c866b5a)</span></li>
<li>autotests/kiotesthelper.h <span style="color: grey">(eb9f0f3019deb63506c2a173d700b78daa95ae10)</span></li>
<li>src/core/CMakeLists.txt <span style="color: grey">(d897e370baedbe06b267934c123acee7a149adff)</span></li>
<li>src/core/chmodjob.cpp <span style="color: grey">(271869bc2a643d715670560b7920efdbc948e560)</span></li>
<li>src/core/job_error.cpp <span style="color: grey">(1551959b6a3cf7060736bea361e840f82651a332)</span></li>
<li>src/core/kfileitem.cpp <span style="color: grey">(7364f87257b5d7dfb760b1c6e5b5d04e1d15a19d)</span></li>
<li>src/core/kioglobal_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/kioglobal_p_unix.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/kioglobal_p_win.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/slave.cpp <span style="color: grey">(787ffcf3cc97a73fb29c2172ed6b8df19ac016fc)</span></li>
<li>src/ioslaves/file/file.h <span style="color: grey">(6477df7cf0d26bf4f581151e1ce8e6c1115a221c)</span></li>
<li>src/ioslaves/file/file.cpp <span style="color: grey">(a642a524c3022ce7f039f90d5bc1f577c88631dc)</span></li>
<li>src/ioslaves/file/file_win.cpp <span style="color: grey">(b0e433e5438e3c45f2f021bf073cb3cca8f4f923)</span></li>
<li>src/ioslaves/ftp/ftp.cpp <span style="color: grey">(79f6144264c03f506309037ed6e8ce429f6c30f0)</span></li>
<li>src/ioslaves/http/http.cpp <span style="color: grey">(de1a1ddde544229689bd22cd69491a46b8c0dddb)</span></li>
<li>src/widgets/CMakeLists.txt <span style="color: grey">(61e4db3566bad08baaa2e7e90b862ddfc8b957f7)</span></li>
<li>src/widgets/config-getgrouplist.h.cmake <span style="color: grey">(6847a19d60be4eb5c2b65fb86258f7368848e6ab)</span></li>
<li>src/widgets/getgrouplist-fake.c <span style="color: grey">(dbe77067371dcedb80cea684fb3cd5f42ed72805)</span></li>
<li>src/widgets/kpropertiesdialog.cpp <span style="color: grey">(8e0a9ba0a806fdb1c9e92de00dfb1c8a1449978c)</span></li>
<li>src/widgets/kurlcompletion.cpp <span style="color: grey">(3f309257c187358de0fd66f9d67f09a712fdf7d6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117078/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>