Review Request: Fix Drkonqi to work with bugzilla 4 (partial port to xml-rpc)

[George Kiagiadakis]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

Review request for KDE Runtime. By George Kiagiadakis. Description This patch partially ports DrKonqi to use xml-rpc, which is done at this point to support the new bugzilla 4.2 instance on bugs.kde.org. I have tried to keep the changes as minimal as possible, so that this can be applied to the 4.8 branch. Note that this patch involves copying the kxmlrpcclient library from kdepimlibs in drkonqi's source code tree. I have done this to avoid modifying dependencies in 4.8.2. However, if an extra dependency of kde-runtime on kdepimlibs sounds ok, I can remove the internal copy in master. Full history at: http://quickgit.kde.org/index.php?p=clones%2Fkde-runtime%2Fgkiagia%2Fdrkonqi.git&a=shortlog&h=refs/heads/drkonqi-xmlrpc Testing Sucessfully reported and attached extra backtrace on: https://bugs4.kde.org/show_bug.cgi?id=294820 https://bugs4.kde.org/show_bug.cgi?id=294821 The duplicate search also seems to be working fine. Bugs: 295276 Diffs drkonqi/CMakeLists.txt (590239dfbd9236463ef11eede699eb84f5806b7a) drkonqi/bugzillalib.h (c89c85c8df3724dc6e51ed03aab7ef7362a22472) drkonqi/bugzillalib.cpp (2df29bd975196b13df02934bfa8899e30d9627f6) drkonqi/drkonqi_globals.h (9281b0574737632782c96f90dadad1c86abebd0c) drkonqi/kxmlrpc/CMakeLists.txt (PRE-CREATION) drkonqi/kxmlrpc/README (PRE-CREATION) drkonqi/kxmlrpc/client.h (PRE-CREATION) drkonqi/kxmlrpc/client.cpp (PRE-CREATION) drkonqi/kxmlrpc/query.h (PRE-CREATION) drkonqi/kxmlrpc/query.cpp (PRE-CREATION) drkonqi/reportassistantpages_bugzilla.cpp (fba91b6d179c867d9984d2296b7302a469c8d0e5) View Diff

[Christoph Feck]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

Great work! Soon the mysterious calmness in bugzilla will be over ;)

drkonqi/bugzillalib.cpp (Diff revision 1)
void BugzillaManager::searchBugsJobFinished(KJob * job)
		317	QString genericError = i18nc("@info", "Received unexpected error code %1 from bugzilla. "
		318	"Error message was: %2", errorCode, errorString);

There are string changes, so the permission from translation team would be needed before this is applied to any branch.

When there are no objections, I suggest to apply this to master to get more testing. Crash reporting does not work right now, so there is nothing that could break.

If possible, this should also be committed to 4.7 branch later, and distributions informed for an eventual online update.

- Christoph

On March 9th, 2012, 12:45 a.m., George Kiagiadakis wrote:

Review request for KDE Runtime. By George Kiagiadakis.

Updated March 9, 2012, 12:45 a.m.

[Kevin Kofler]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

I think adding a dependency is a much cleaner solution than bundling. But I wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to kdelibs, it clearly seems to be useful for more than just PIM. But in distros, I think most of us prefer the dependency on kdepimlibs (we can also make a subpackage for that library in our packaging) to bundling (which is against Fedora and Debian policies, at least).

- Kevin

On March 9th, 2012, 12:45 a.m., George Kiagiadakis wrote:

Review request for KDE Runtime. By George Kiagiadakis.

Updated March 9, 2012, 12:45 a.m. Description

[Michael Pyne]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 2:23 p.m., Kevin Kofler wrote:

I think adding a dependency is a much cleaner solution than bundling. But I wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to kdelibs, it clearly seems to be useful for more than just PIM. But in distros, I think most of us prefer the dependency on kdepimlibs (we can also make a subpackage for that library in our packaging) to bundling (which is against Fedora and Debian policies, at least).

I would also agree with moving the functionality of kxmlrpcclient to kdelibs if it is to be used by something as core as the bug reporter (and is useful generically beyond that nowadays, many more things use XML-RPC web services than just Kolab. Plasma applets could use this as well for instance).

I'm not sure it's a good idea to do this move in the 4.7 branch of kdelibs though... is there a way to have kdelibs-4.7 and kdepimlibs-4.8 *both* include the class (with kdepimlibs-4.8 using a weak symbol or something similar to defer to the kdelibs-4.7 one if present)? If that's at all difficult I'd say just add the short-term dependency on kdepimlibs and be done with it.

- Michael

[David Faure]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 2:23 p.m., Kevin Kofler wrote:

I think adding a dependency is a much cleaner solution than bundling. But I wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to kdelibs, it clearly seems to be useful for more than just PIM. But in distros, I think most of us prefer the dependency on kdepimlibs (we can also make a subpackage for that library in our packaging) to bundling (which is against Fedora and Debian policies, at least).

On March 9th, 2012, 6:21 p.m., Michael Pyne wrote:

I would also agree with moving the functionality of kxmlrpcclient to kdelibs if it is to be used by something as core as the bug reporter (and is useful generically beyond that nowadays, many more things use XML-RPC web services than just Kolab. Plasma applets could use this as well for instance).

I'm not sure it's a good idea to do this move in the 4.7 branch of kdelibs though... is there a way to have kdelibs-4.7 and kdepimlibs-4.8 *both* include the class (with kdepimlibs-4.8 using a weak symbol or something similar to defer to the kdelibs-4.7 one if present)? If that's at all difficult I'd say just add the short-term dependency on kdepimlibs and be done with it.

You mean kdelibs-4.8, not 4.7.

But 4.8.0 and 4.8.1 have been released already, so IMHO this is something for 4.9.
Meanwhile we can use the duplication solution from this patch.

- David

[George Kiagiadakis]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 2:23 p.m., Kevin Kofler wrote:

I think adding a dependency is a much cleaner solution than bundling. But I wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to kdelibs, it clearly seems to be useful for more than just PIM. But in distros, I think most of us prefer the dependency on kdepimlibs (we can also make a subpackage for that library in our packaging) to bundling (which is against Fedora and Debian policies, at least).

On March 9th, 2012, 6:21 p.m., Michael Pyne wrote:

I would also agree with moving the functionality of kxmlrpcclient to kdelibs if it is to be used by something as core as the bug reporter (and is useful generically beyond that nowadays, many more things use XML-RPC web services than just Kolab. Plasma applets could use this as well for instance).

I'm not sure it's a good idea to do this move in the 4.7 branch of kdelibs though... is there a way to have kdelibs-4.7 and kdepimlibs-4.8 *both* include the class (with kdepimlibs-4.8 using a weak symbol or something similar to defer to the kdelibs-4.7 one if present)? If that's at all difficult I'd say just add the short-term dependency on kdepimlibs and be done with it.

On March 9th, 2012, 6:41 p.m., David Faure wrote:

You mean kdelibs-4.8, not 4.7.

But 4.8.0 and 4.8.1 have been released already, so IMHO this is something for 4.9.
Meanwhile we can use the duplication solution from this patch.

Yes, as I said I did this bundling only to avoid doing big changes in the 4.8 branch. In 4.9 I agree that we should handle this differently, either move kxmlrpcclient to kdelibs or make kde-runtime depend on kdepimlibs.

- George

[George Kiagiadakis]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 11:31 a.m., Christoph Feck wrote:

When there are no objections, I suggest to apply this to master to get more testing. Crash reporting does not work right now, so there is nothing that could break.

If possible, this should also be committed to 4.7 branch later, and distributions informed for an eventual online update.

Yes, there is one new string, but it's worth it. We can live with it even untranslated, since it is only shown on error conditions.

I intend to apply this both in 4.8 and master at the same time. (i.e. apply to 4.8 and merge 4.8 to master... or is that still not the way to do it?)

If there is need for it in 4.7, I'll happily backport it. It should apply just fine actually, I have no reason to believe that it doesn't.

- George

[Oswald Buddenhagen]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 11:31 a.m., Christoph Feck wrote:

When there are no objections, I suggest to apply this to master to get more testing. Crash reporting does not work right now, so there is nothing that could break.

If possible, this should also be committed to 4.7 branch later, and distributions informed for an eventual online update.

On March 10th, 2012, 12:29 a.m., George Kiagiadakis wrote:

Yes, there is one new string, but it's worth it. We can live with it even untranslated, since it is only shown on error conditions.

I intend to apply this both in 4.8 and master at the same time. (i.e. apply to 4.8 and merge 4.8 to master... or is that still not the way to do it?)

If there is need for it in 4.7, I'll happily backport it. It should apply just fine actually, I have no reason to believe that it doesn't.

> (i.e. apply to 4.8 and merge 4.8 to master... or is that still not the way to do it?)
>
only kdelibs is merged 4.8 => frameworks. everything is else is cherry-picked as ever since.

- Oswald

[Christoph Feck]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 2:23 p.m., Kevin Kofler wrote:

I think adding a dependency is a much cleaner solution than bundling. But I wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to kdelibs, it clearly seems to be useful for more than just PIM. But in distros, I think most of us prefer the dependency on kdepimlibs (we can also make a subpackage for that library in our packaging) to bundling (which is against Fedora and Debian policies, at least).

On March 9th, 2012, 6:21 p.m., Michael Pyne wrote:

I would also agree with moving the functionality of kxmlrpcclient to kdelibs if it is to be used by something as core as the bug reporter (and is useful generically beyond that nowadays, many more things use XML-RPC web services than just Kolab. Plasma applets could use this as well for instance).

I'm not sure it's a good idea to do this move in the 4.7 branch of kdelibs though... is there a way to have kdelibs-4.7 and kdepimlibs-4.8 *both* include the class (with kdepimlibs-4.8 using a weak symbol or something similar to defer to the kdelibs-4.7 one if present)? If that's at all difficult I'd say just add the short-term dependency on kdepimlibs and be done with it.

On March 9th, 2012, 6:41 p.m., David Faure wrote:

You mean kdelibs-4.8, not 4.7.

But 4.8.0 and 4.8.1 have been released already, so IMHO this is something for 4.9.
Meanwhile we can use the duplication solution from this patch.

On March 10th, 2012, 12:16 a.m., George Kiagiadakis wrote:

Yes, as I said I did this bundling only to avoid doing big changes in the 4.8 branch. In 4.9 I agree that we should handle this differently, either move kxmlrpcclient to kdelibs or make kde-runtime depend on kdepimlibs.

There were compatibility fixes in bugs.kde.org, so do we still need this? I guess using xmlrpc is the more correct way to do it, so we don't break it again later, but maybe this can be postponed to 4.9+ only.

- Christoph

[George Kiagiadakis]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 2:23 p.m., Kevin Kofler wrote:

I think adding a dependency is a much cleaner solution than bundling. But I wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to kdelibs, it clearly seems to be useful for more than just PIM. But in distros, I think most of us prefer the dependency on kdepimlibs (we can also make a subpackage for that library in our packaging) to bundling (which is against Fedora and Debian policies, at least).

On March 9th, 2012, 6:21 p.m., Michael Pyne wrote:

I would also agree with moving the functionality of kxmlrpcclient to kdelibs if it is to be used by something as core as the bug reporter (and is useful generically beyond that nowadays, many more things use XML-RPC web services than just Kolab. Plasma applets could use this as well for instance).

I'm not sure it's a good idea to do this move in the 4.7 branch of kdelibs though... is there a way to have kdelibs-4.7 and kdepimlibs-4.8 *both* include the class (with kdepimlibs-4.8 using a weak symbol or something similar to defer to the kdelibs-4.7 one if present)? If that's at all difficult I'd say just add the short-term dependency on kdepimlibs and be done with it.

On March 9th, 2012, 6:41 p.m., David Faure wrote:

You mean kdelibs-4.8, not 4.7.

But 4.8.0 and 4.8.1 have been released already, so IMHO this is something for 4.9.
Meanwhile we can use the duplication solution from this patch.

On March 10th, 2012, 12:16 a.m., George Kiagiadakis wrote:

Yes, as I said I did this bundling only to avoid doing big changes in the 4.8 branch. In 4.9 I agree that we should handle this differently, either move kxmlrpcclient to kdelibs or make kde-runtime depend on kdepimlibs.

On March 13th, 2012, 12:31 p.m., Christoph Feck wrote:

There were compatibility fixes in bugs.kde.org, so do we still need this? I guess using xmlrpc is the more correct way to do it, so we don't break it again later, but maybe this can be postponed to 4.9+ only.

Yes, we don't need it anymore in the 4.8 branch. This is 4.9+ now. What remains to be seen is what to do with kxmlrpcclient...

- George

[Milian Wolff]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104200/

On March 9th, 2012, 2:23 p.m., Kevin Kofler wrote:

I think adding a dependency is a much cleaner solution than bundling. But I wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to kdelibs, it clearly seems to be useful for more than just PIM. But in distros, I think most of us prefer the dependency on kdepimlibs (we can also make a subpackage for that library in our packaging) to bundling (which is against Fedora and Debian policies, at least).

On March 9th, 2012, 6:21 p.m., Michael Pyne wrote:

I would also agree with moving the functionality of kxmlrpcclient to kdelibs if it is to be used by something as core as the bug reporter (and is useful generically beyond that nowadays, many more things use XML-RPC web services than just Kolab. Plasma applets could use this as well for instance).

I'm not sure it's a good idea to do this move in the 4.7 branch of kdelibs though... is there a way to have kdelibs-4.7 and kdepimlibs-4.8 *both* include the class (with kdepimlibs-4.8 using a weak symbol or something similar to defer to the kdelibs-4.7 one if present)? If that's at all difficult I'd say just add the short-term dependency on kdepimlibs and be done with it.

On March 9th, 2012, 6:41 p.m., David Faure wrote:

You mean kdelibs-4.8, not 4.7.

But 4.8.0 and 4.8.1 have been released already, so IMHO this is something for 4.9.
Meanwhile we can use the duplication solution from this patch.

On March 10th, 2012, 12:16 a.m., George Kiagiadakis wrote:

Yes, as I said I did this bundling only to avoid doing big changes in the 4.8 branch. In 4.9 I agree that we should handle this differently, either move kxmlrpcclient to kdelibs or make kde-runtime depend on kdepimlibs.

On March 13th, 2012, 12:31 p.m., Christoph Feck wrote:

There were compatibility fixes in bugs.kde.org, so do we still need this? I guess using xmlrpc is the more correct way to do it, so we don't break it again later, but maybe this can be postponed to 4.9+ only.

On March 13th, 2012, 1:26 p.m., George Kiagiadakis wrote:

Yes, we don't need it anymore in the 4.8 branch. This is 4.9+ now. What remains to be seen is what to do with kxmlrpcclient...

note that I still want to write a bugzilla fatclient in the future. while linking against kdepimlibs wouldn't be a problem for me there, having kxmlrpcclient in kdelibs wouldn't hurt either.

- Milian