Code review comment for lp://staging/~kalikiana/ubuntu-ui-toolkit/appsettings

> > Will the Settings.gropup property change cause settings reload?
>
> No and I don't see a use case for changing groups at runtime, so I added a
> warning that's shown if you try.

A use case is when you define different profiles for instance. Assume you have a host/port/login/password quadruple for "default", "home", "other device" groups. Then in your app you select the setting to be loaded based on the group itself. I'm talking about the settings use, not about its declaration.

>
> > 735 +Item {
> > 736 + id: settings
> > Do we need an Item for this? In that way we can put visual items onto it,
> but that does not make any sense... Shouldn't we use Object instead?
>
> Yes and no. It must be an Item so that you can put it into another Item. It
> doesn't say anything about what you can put inside.

You can put QtObjects inside Items, not only Items. The danger in deriving from Item is that you can declare visual components inside the Settings upon use, which may cause problems (as Settings' default w/h is 0).

I'd rather go for QtObject using the same approach for children definition as we have in Object.

>
> > 753 + Component.onCompleted: {
> > What if children get changed after this point? Do we exclude them from state
> saving?
>
> I changed it, this didn't work before I ported to a custom default property.
>
> > default property list<Option> options
> > So we make sure we cannot add anything else into the Settings blob
>
> I failed to make this work before, I gave it another try. I managed to get it
> to work by using an alias on a second property which is the real list. If I
> declare it as one I get syntax errors. And indeed with this I could drop the
> hasOwnProperty checks. Though the error message generated by QML is not really
> great, it gets us an early failure.

Yep, that's the way to do it, just like we have in Object. Btw, you could make that aliased property internal, as the name is not really nice :D

>
> > Please close each JS line with semicolon.
>
> Done.

Cool.