API review: FavIconManager vs FavIconHostRequestJob

[David Faure]

On Sunday 31 January 2016 11:29:31 laurent Montel wrote:
> Le dimanche 31 janvier 2016, 10:40:12 CET David Faure a écrit :
> > On Thursday 28 January 2016 09:35:53 laurent Montel wrote:
> > > +ecm_qt_declare_logging_category(kiogui_SRCS HEADER favicons_debug.h
> > > IDENTIFIER FAVICONS_LOG CATEGORY_NAME kde.kio.favicons)
> > > In this directory we will use just favicon ?
> > > Perhaps a more generic log name no ?
> > 
> > Well, one debug area per lib is the lazy solution. More precise debug areas
> > allow to turn on favicon debugging without turning on a whole lot of other
> > stuff.
> 
> For me too, but as you create an unique ecm_qt_declare_logging_category for 
> this lib I thought that you wanted to create just one debug area.
> => you will add other ecm_qt_declare_logging_category for each new class in 
> kiogui lib ?

Each new "area" yes (set of classes around the same feature).

> > > For me api seems good.
> > 
> > Which one? The Manager API or the Job API? :-)
> 
> I spoke about Manager I prefere this one.

I wonder why, other than possibly smaller porting effort?

The more I think about it, the less I like the Manager API.
See all the stuff about "dicts of listeners" in akregator?
It can all go away when using jobs. Instead of registering a listener
for a given host icon, and then the manager having to look up who is
registered... the code could just create job.

It's not like akregator was even using the manager in order to
avoid downloading the same icon twice at the same time
when 2 widgets request it at the same time. addListener
called loadIcon, which triggered a download.
Anyway, I can implement the sharing of downloads behind
the scenes if we need to, although now I'm wondering if we
need to, given that the old code didn't do it (AFAICS).
 
> Keditbookmarks is not released, konqueror is not.

That doesn't matter to me, I'll port them anyway :-)

> Rss dataengine I don't know
> Akregator I can port it :)
> So really just rss dataengine + akregator needs to be ported.

Well, they all do, but that's fine.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5