extender api review round 3

[Richard Moore]

I've snipped out the parts where there's nothing to add:

On 8/1/08, Rob Scheepmaker <r.scheepmaker at student.utwente.nl> wrote:
> > should saveState() be public? Why no restoreState()?
> actually... yeah, saveState could be protected. restoreState isn't needed because in itemAddedEvent you
> can read the stuff you stored in saveState and use it to add items correctly.

Making it protected sounds good - public implies that anyone is free
to call it. It might be worth adding a note in the docs about how the
state should be restored.

> >
> > ExtenderItem
> > ==========
> >
> > KConfigGroup config();
> > should this be const?
>
> ehm, no, Applets should be able to write stuff to this config group.

Ok.

> > void destroy(); Needs better API docs. How does this differ from
> > simply deleting the object?
>
> That it also remove it's config group, instead  of just deleting the object.

Does it do both? ie. delete the object and its config group. If not
then a method name that reflects the fact it just deletes the config
and relies on the destruction for the rest would be a good idea.

> > Applet
> > =====
> >
> > virtual void initExtenderItem(ExtenderItem *item);
> >
> > This api feels a bit of kludge. As documented it sounds like an API
> > for dealing with abnormal failures like crashes. These should be
> > avoided rather than coded for. Am I missing something?
>
> Actually it's not for dealing with crashes, but for being able to create the extender items again after
> you've restarted plasma. Even after you reboot your computer you still want extender items that you've
> detached previously to be available.

Yeah, I still don't get that. Why would this information not be saved
when the items were created? What is special enough that we need a
call for this. I'm not saying you're wrong, but it's not clear to me
why this should be needed in normal operation.

Cheers

Rich.


> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel at kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>