> On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: > > drkonqi/bugzillalib.cpp, line 81 > > <https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line81> > > > > The patch largely consists of hand-crafted version handling. > > > > replacing this by "int version = KDE_MAKE_VERSION(major, minor, > > release);" and simple integer metricts for comparism should considerably > > lower complexity (thus make the patch easier to be accepted ;-) > > > > Yes, I know it's crap to write a lot of code and remove it afterwards.
This is exactly the kind of advice I hope for in a review. I did look to see, several weeks ago, if there were some version-compare methods in KDE or Qt or on Google, but did not find any. I also considered something like version = (major * 1000000 + minor * 1000 + release), but thought it would be rather a 1960s style kludge... ;-) I have lived and worked through all the trouble those YYMMDD strings caused (Y2K and all that). Still, I suppose the parts of Bugzilla versions are unlikely to go past 255... I have no objection to deleting code and replacing it with something shorter and simpler. I guess I would still need the code to convert a QString version (from Bugzilla and the bugs.kde.org database) to a single KDE_MAKE_VERSION integer, or is there something else in KDE/Qt to do that? What *does* worry me is "5-minutes-to-midnight" re-programming, i.e. I would have to make and test all changes on Thursday, just hours before Albert starts tagging KDE 4.14.2. In my experience, that could be riskier than committing my patch as it stands. But either way, we at least have a month to fix any problems before KDE 4.14.3, the last release of KDE 4. Then there is always KDE 14.12 and Dr Konqi is an app, not a library... @Albert and Thomas: Please let me know what you would like me to commit on Thursday: my patch as it stands, my patch simplified as Thomas suggests or Frédéric's patch. > On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: > > drkonqi/bugzillalib.cpp, line 131 > > <https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line131> > > > > Afaiu the bugzilla roadmap, they become mandatory with 5.0.0, not > > 4.5.0? (only deprecated w/ 4.5 but this would add more time to invoke > > kwallet etc.) AFAICS Bugzilla doco writers sometimes use 5.0 as a shorthand for 4.5.0. See REFERENCES: in the Description section of this review, starting at "api" in the top Bugzilla doco webpage. Confusing, isn't it? The Bugzilla version after 4.5.x might be 4.6.x or 5.0.x. Who can tell? - Ian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 ----------------------------------------------------------- On Oct. 7, 2014, 7:42 a.m., Ian Wadham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120431/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2014, 7:42 a.m.) > > > Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío > Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. > > > Bugs: 337742 > http://bugs.kde.org/show_bug.cgi?id=337742 > > > Repository: kde-runtime > > > Description > ------- > > When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security > method used by Bugzilla changed from cookies to tokens that had to be > supplied as parameters with every secure remote-procedure call. Further > changes to security methods have been announced by Bugzilla and are > documented for unstable 4.5.x versions of Bugzilla software. Tokens will be > deprecated and then discontinued. When this happens, Dr Konqi will need to > supply a user-login name and a password with every secure remote-procedure > call. Furthermore, the traditional "User.login" call presently used by Dr > Konqi will be deprecated and discontinued. > > This patch fixes the tokens problem, which has given rise to several bug > reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also > provides for automatic switching to passwords-only security as and when the > Bugzilla version changes again. This uses > a general data-driven approach which can be easily updated, ahead of time, > next time Bugzilla announces a change that affects Dr Konqi, whether it be in > security methods or some other feature. > > NOTES: > 1. This patch is intended to be forward-portable to Frameworks/KF5, but I > work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do > the porting and testing. So could someone else please do it? > 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses > the tokens issue only, but it should be reviewed and shipped as a matter of > urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE > 4.14 being due for tagging on Thursday, 9 October. That will leave more time > for this review (120431) of my more long-term and more general patch. > 3. The passwords-only part of my patch is currently storing the password in > clear. Suggestions re encryption are welcomed --- or the code could be > changed to make use of KWalletD mandatory (but that might not be fully > portable to all platforms). > 4. When the Bugzilla call "User.login" is discontinued, some re-sequencing of > the flow of KAssistantDialog pages will be needed. I have not attempted to do > that at this stage. Probably the entry of the user name and password should > be delayed until the report has been accepted by the Dr Konqi logic and it is > just about to be sent to bugs.kde.org or attached to an existing bug report. > > REFERENCES: > http://www.bugzilla.org/docs/ > http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN > Bugzilla 4.5.x (future) API doco re security > http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN > Bugzilla 4.4.5 (current) API doco re security > http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login > User.login will be DEPRECATED in 4.5.x > > > Diffs > ----- > > drkonqi/bugzillalib.h 570169b > drkonqi/bugzillalib.cpp f74753c > drkonqi/reportassistantpages_bugzilla.h b7af5b8 > drkonqi/reportassistantpages_bugzilla.cpp 22183f0 > > Diff: https://git.reviewboard.kde.org/r/120431/diff/ > > > Testing > ------- > > Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime > repository. > > Tested a range of version numbers (see commented-out test data) against a > range of 5 or 6 hypothetical and real Bugzilla versions at which things could > or will change. This was to test the basic version-checking and > feature-choosing algorithm. > > Tested submitting both full reports and attached reports, using both the > token method and the passwords-only method. > > Also tested with KWalletD supplying the username and password on Dr Konqi's > login dialog. > > > Thanks, > > Ian Wadham > >
