<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119805/">https://git.reviewboard.kde.org/r/119805/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 16th, 2014, 7:46 p.m. UTC, <b>Denis Steckelmacher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Great work! I've tested this patch in this configuration and it works: no PySVN, SVN installed, Python 2. I have one question, though: why do you try to get the temporary path for the cached file using three different OS-dependent methods while you have an OS-independent fallback? Why not directly use tempfile.gettempdir?</p></pre>
</blockquote>
<p>On August 16th, 2014, 8:20 p.m. UTC, <b>Alex Merry</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I wanted to avoid using a temporary directory, as this is really a caching thing (temporary directories are generally intended for things that won't last longer than a single execution of the program).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Admittedly, it's debatable in this case, since we're not maintaining it for more than an hour (although it's still useful to keep around for longer in case you lose the internet connection).</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, I understand, but as you point out, 1 hour is a very short delay and I don't think that anybody has its /tmp directory automatically cleaned after one hour. If I remember well, /tmp is generally not cleaned before a computer reboot (it is either stored in RAM, or a shutdown script removes everything in it).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">On the other hand, api.kde.org is generated only once a day, and most maintainers are "stable" and will not change name/email every hour. So, I see two solutions:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Keep the one hour delay but put everything in /tmp</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Keep your cache paths but change the delay so that one day (or two) are allowed to pass before the file is re-downloaded.</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It is also possible to use /tmp and a big delay, which is the simplest solution, but then some people will download the file a bit more often than expected because they have rebooted their computer. But in a world where people don't reboot their computer but put it to sleep, I don't think that this is really a problem.</p></pre>
<br />
<p>- Denis</p>
<br />
<p>On August 16th, 2014, 3:29 p.m. UTC, Alex Merry wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, Denis Steckelmacher and Aurélien Gâteau.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated Aug. 16, 2014, 3:29 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kapidox
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">WebSVN is not set up to deal with a large number of hits - it is<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
intended for casual browsing. "svn export" is much more efficient, and<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
the anonsvn servers are meant to take this load.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Given that we would like developers to be generating documentation to<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
check their doxygen markup, we should use a method that scales.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This uses "svn export" (either through libsvn python bindings or the<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
command-line utility) if it can. It also caches the result for an hour<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
(since the accounts file is only updated server-side once an hour<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
anyway).</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Extracted download_kde_identities() function (and its dependent functions) to another script, and tested with python2 (with libsvn bindings), python3 (without) and python3 (with the svn command line altered so that it would fall back to websvn) - both with and without a cache file/cache directory present.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also ran kgenframeworksapidox (python2), and checked that David Faure was correctly listed as the maintainer of KArchive, with an email address.</p></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>src/kapidox/generator.py <span style="color: grey">(ba4fad92d4b027d791107c58b15b26187d6f3087)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119805/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>