Export the APIs needed to build Blogilo again
AbandonedPublic

Authored by mlaurent on Jan 3 2019, 6:57 PM.

Details

Reviewers
kkofler
Group Reviewers
KDE PIM
Summary

This undoes parts of Laurent Montel's export cleanups, because those 4
classes are needed to build (the last release of) Blogilo:

  • src/CMakeLists.txt: Install the InsertHtmlDialog headers (inserthtmldialog.h, InsertHtmlDialog).
  • src/emoticontexteditaction.h: Export the KPIMTextEdit::EmoticonTextEditAction class.
  • src/inserthtmldialog.h: Use the public export macros for the KPIMTextEdit::InsertHtmlDialog class.
  • src/insertimagewidget.h: Export the KPIMTextEdit::InsertImageWidget class.
  • src/inserttablewidget.h: Export the KPIMTextEdit::InsertTableWidget class.

This allows building Blogilo again.

Test Plan

Diff Detail

Repository
R86 PIM: Text Editor
Lint
Lint Skipped
Unit
Unit Tests Skipped
kkofler created this revision.Jan 3 2019, 6:57 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptJan 3 2019, 6:57 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
kkofler requested review of this revision.Jan 3 2019, 6:57 PM
mlaurent requested changes to this revision.Jan 3 2019, 8:03 PM

Blogilo is dead from very long term.
So sorry I am against this patch.
Regards.

This revision now requires changes to proceed.Jan 3 2019, 8:03 PM

Exporting these classes is all that is needed to keep Blogilo working. There is no practical reason to break backwards compatibility of the KPIMTextEdit library that way, your only rationale for removing the exports is that they "should not be needed". The classes still exist either way.

Or are there different interfaces that Blogilo should be using instead? If so, which are they? There is no documentation on that.

kkofler requested review of this revision.Jan 3 2019, 9:05 PM

As I told you blogilo is dead from long time.
If you want to continue to distribut it in your distro apply this patch.
But as blogilo is broken I will not accept your patch.

That's all for me.
Regards.

Blogilo is dead and no longer even offered as part of the Applications releases, and I doubt it does even work nowadays (at least not with WP since IIRC the XMLRPC interface is disabled by default or even removed). Exactly what are you trying to accomplish?

kkofler added a comment.EditedJan 3 2019, 9:54 PM

I am trying to resurrect Blogilo in Fedora. I am upgrading from the EOL Fedora 27 to the supported Fedora 28 (and eventually 29) and noticed that Blogilo was going away. So I tracked it down to the build failure caused by this export removal. I already got it building in my Kannolo Copr repository.

Blogilo still works for wordpress.com, proof: https://kevinkofler.wordpress.com/2019/01/03/blogilo-test-blog-entry/

Sure, we can apply this patch downstream (and Rex Dieter already OKed it downstream), but it would make our lives easier if this trivial change could just be applied upstream.

Then please do this downstream. If it would benefit anything else than Blogilo I would be in principle OK, but it doesn't make sense for upstream. (In openSUSE we removed the package, IIRC).

@kkofler so blogilo is removed from kdepim team, that means like other expressed, that we would not put effort in keeping it running. As we are already exhausted by the amount of application under our umbrella and also blogilo is not really in the center of a pim suite...
On the other side you got blogilo built with a small patch, great. So if you care about blogilo feel welcome to take over mantainership for blogilo!
Than we surely find also solutions how we handle this patch. We simple want to make sure, that software we ship is in a good shape. And exporting symbols for an unmaintained application does not make sense, otherwise kdepim is getting a dumping ground for rotten applications.
If you have any questions regarding, what it means to take over mantainership or anything else feel free to ask. Maybe the best place is our mailinglist: kde-pim@kde.org

"On the other side you got blogilo built with a small patch, great. So if you care about blogilo feel welcome to take over mantainership for blogilo!" building doesn't mean that it works.
And I know that it"s totally broken.

So for me until it will not maintain and fixed I will not accept this patch.
Thanks if you will fix blogilo but until it this patch will not accepting.

Regards

mlaurent commandeered this revision.Sat, Mar 23, 8:10 AM
mlaurent abandoned this revision.
mlaurent edited reviewers, added: kkofler; removed: mlaurent.

It will never apply. I close it.
Regards

When you look at how simple and uninvasive the change is (it just exports code that is there anyway) and that it actually only restores backwards source and binary compatibility (which is normally considered a good thing in KDE land, it would even be mandatory if this were an official KDE Framework), I think you (all three) are really being unhelpful and uncooperative here.

dvratil added a subscriber: dvratil.EditedSat, Mar 23, 4:23 PM

@mlaurent this change actually fixes a bug in the library - InsertTableWidget and InsertImageWidget header files are installed so the classes are considered public and must be thus exported - which makes me think you only looked at the "Blogilo" in the title without looking at the code itself, which would be rather disappointing. If you disregard the fact that this is a fix to keep Blogilo compiling, but take it as a general improvement to the library, why not accept it? Would you accept this patch had Kevin not mentioned Blogilo at all, just called it "add missing exports"? Yes, Blogilo is not maintained and released by us, but it's not fair to actively and intentionally prevent people from keeping it working for themselves by not accepting literally harmless patches - this is not how we roll in KDE at all.

For me fixing lib is not installed headers as it's not used by any program.
I don't consider blogilo as a program as it's not maintain it's broken (I broken it when I ported to webengine and I signaled it as it was broken long time ago).

So I can accept a patch for not installing some not exported class but not exported some symbol which will not used.

I don't see the problem if this patch is applying in fedora package but I don't see the useful for kpim as it never used.