<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/120876/">https://git.reviewboard.kde.org/r/120876/</a>
</td>
</tr>
</table>
<br />
<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;">The following ideas are concerns about design, rather than coding.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I do not know what the underlying objectives of Frameworks are, including who is intended to use it and for what, so I will be guided by you and Thomas and the Frameworks team.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">IF Frameworks is for use in KDE software only and IF the bugs.kde.org database is the same for all users of KDE software world-wide (both KDE 4 and KF 5) and will never go back to using cookies, THEN you can actually dispense with all the Bugzilla version-checking code in the current patch and all the security methods except for "UsePasswords". This is because "UsePasswords" has actually been supported since Bugzilla version 3.2 (i.e. since before the "UseCookies" version of Bugzilla [4.3.x] which KDE software had in June/July and before).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I spotted this potential simplification late in the https://git.reviewboard.kde.org/r/120431/ process: too late to bring it up at that point. It is, in fact, why I was able to test "UsePasswords" mode... :-)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">OTOH, IF Frameworks is also for use in non-KDE software (presumably based on Qt), THEN you need to keep the version checking, because you probably cannot know what database and Bugzilla version that other software might prefer to use. This scenario can happen. I saw an enquiry by a FOSS developer only recently (on one of the Apple FOSS lists I think). He liked KCrash and Dr Konqi, wanted to use them in his own software package and was asking what might be involved.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In both potential uses of Frameworks, you can dispense with the "UseTokens" security method altogether and go straight to "UsePasswords". At first reading, I thought that was what you had done in your original patch, Hrvoje... :-) So you could have "UseCookies" and "UsePasswords" --- or just "UsePasswords".</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also you can, I think, drop the call to Bugzilla's login when using "UsePasswords" mode. Support for login calls will be discontinued in a future version of Bugzilla (after 4.5.0). I do not think you need a login call if you are sending a user name and password with every database update call. If I am right about that, dropping the login call will give Dr K even more future-proofing against currently announced Bugzilla changes. Of course, you will still need a login dialog or some way to get a username and password, from KWallet or whatever...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I wrote a message about this stuff and it was forwarded to the k-f-d list recently (I am not subscribed there). Did you see it?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You might also be able to strip out the Dr Konqi code that checks if the KCookieJar is available, or make it conditionally compiled. You could avoid a few more of Dr Konqi's dependencies that way.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Finally, whatever you decide, it would be good to keep the basics of the Bugzilla-version-checking code, in case of other announced changes to Bugzilla software in the future.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I am thinking particularly of changes to the database schema, call parameters, return values, etc. as Bugzilla evolves.</p></pre>
<br />
<p>- Ian Wadham</p>
<br />
<p>On October 29th, 2014, 8:41 p.m. UTC, Hrvoje Senjan 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 Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.</div>
<div>By Hrvoje Senjan.</div>
<p style="color: grey;"><i>Updated Oct. 29, 2014, 8:41 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=337742">337742</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</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;">discussion was in https://git.reviewboard.kde.org/r/120431/
removed the version checks, as we know we have kdelibs >= 4.5 ;-)</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;">builds, succesfully reported bug via patched DrKonqi, wasn't able to do so before.</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>drkonqi/bugzillaintegration/bugzillalib.h <span style="color: grey">(570169b)</span></li>
<li>drkonqi/bugzillaintegration/bugzillalib.cpp <span style="color: grey">(8fd8399)</span></li>
<li>drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h <span style="color: grey">(50cf05f)</span></li>
<li>drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp <span style="color: grey">(5a6096f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120876/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>