-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/#review69516
-----------------------------------------------------------


The following ideas are concerns about design, rather than coding.

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.

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).

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... :-)

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.

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".

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...

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?

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.

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.

I am thinking particularly of changes to the database schema, call parameters, 
return values, etc. as Bugzilla evolves.

- Ian Wadham


On Oct. 29, 2014, 8:41 p.m., Hrvoje Senjan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120876/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2014, 8:41 p.m.)
> 
> 
> Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.
> 
> 
> Bugs: 337742
>     https://bugs.kde.org/show_bug.cgi?id=337742
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> discussion was in https://git.reviewboard.kde.org/r/120431/
> removed the version checks, as we know we have kdelibs >= 4.5 ;-)
> 
> 
> Diffs
> -----
> 
>   drkonqi/bugzillaintegration/bugzillalib.h 570169b 
>   drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
>   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
>   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 
> 
> Diff: https://git.reviewboard.kde.org/r/120876/diff/
> 
> 
> Testing
> -------
> 
> builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
> before.
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to