Code review comment for lp:~fboucault/unity-2d/dconf_migration_super_key

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

> > compile and works well.
> >
> > However, we try now to enforce sane dconf values, and not only for desrt ;)
> > so, can you please use as a gsettings path some /com/canonical/ one? instead
> > of /desktop/unity-2d (it should match the id com.canonical.* for non
> > relocatable schemas)
> >
>
> I was not sure so I followed what Unity was doing. Should I definitely do what
> you suggest?
Yeah, Unity will change that in the coming release

>
> >
> > Maybe a stupid question, but shouldn't updateSuperKeyMonitoring() SLOT take
> > the bool parameter rather than asking dconf again about which value for this
> > property?
> >
>
> Not a stupid question. I wanted the change to be minimal and strictly about
> migrating to dconf so I kept it as is.

Ok, nice enhancement target for the future then?
>
> > Apart from that, all looks good, it works and changing the value in dconf-
> > editor has indeed some effect. Nice work :)
>
> Thanks for the testing!

So, approved with once the change #1 is done :)

review: Approve

« Back to merge proposal