Code review comment for lp:~unity-team/unity8/launcher-backend

W dniu 08.07.2013 16:11, Michael Zanetti pisze:

>> 205 +LauncherBackend::~LauncherBackend()
>> 206 +{
>> 207 +
>> 208 +}
>>
>> Is it just a template thing, or do we want empty destructors as a rule?
>
> It's mostly just a template thing... Can remove it if you want, but I'm quite sure Antti will need it again.

OK.

>> Hmm hmm... Why doesn't LauncherBackend have an API definition in lp:unity-api?
>> Or is it just a stop-gap? Same for QuikcListEntry? Or is it even just a thing
>> that the shell won't ever see?
>
> Exactly... Thats the idea. Both are an implementation detail of me and Antti but never show up in the public QML api.

Yeah, but then... If we wanted to mock one or the other... Will all of
that later get away into a separate "unity-launcher" project for the
backend?

>> 355 + QStringList storedApplications() const;
>>
>> Shouldn't this be "pinnedApplications"?
>
> No, because this does not only contain pinned apps. It also contains recent ones. So if you look at it from the backend perspective, this is whant needs to be stored into the settings. The decision if its pinned, running, recent, whatever happens one layer above in LauncherModel.

Why should recent be stored? Info about recent apps should come from
Zeitgeist, and be volatile, no?

>> 401 + //TODO: Fill in getters for all the other things needed from the
>> .desktop file.
>>
>> I'm not sure that's needed, we should have a generic "AppData" component or
>> similar that would take an appId and expose all the data read from .desktop
>> and potentially other places.
>
> Yeah... Not really sure how the desktop file parser will work yet. In general I would leave that to Antti to finish up the backend API as he sees fit.

Let's just make sure we don't need to learn different APIs wherever
we're looking at an app.

>> One think we're missing is the Gettext domain to use to translate the strings.
>> It could be considered a backend task, though.
>
> yeah. I expect the backend to deliver translated app names so far. It would be quite easy to change it turns out to be a bad idea... But thats it for now and both, me and Antti where happy with it.

Yeah, I think that's common with what we do/expect in the other backends.

--
Michał (Saviq) Sawicz <email address hidden>
Canonical Services Ltd.