[Merge] ~enr0n/ubuntu-release-upgrader:ubuntu/main into ubuntu-release-upgrader:ubuntu/main

Nick Rosbrook mp+452304 at code.launchpad.net
Thu Sep 28 13:42:53 UTC 2023


> Thanks for that explanation. As regards Gio I see that you used
> "settings.sync()" which I didn't. Maybe that's it. Or not, see below.
> 
> I have a couple of observations on the part of the code I understand. :/
> 
> The partition() method does not safely split the value into font family and
> size. It ought to work for e.g. 'Ubuntu 11', since the family is one single
> word, but it didn't work for me when testing. I had set 'Ubuntu 14' and it
> switched to 'Sans 11'.

Ah yes, this should be changed. I will test the font size issue, but obviously that's not the intention.
> 
> => This makes we wonder if the fetching of the user value is at all
> successful.
> 
> (And it would safely fail if the user value was e.g. 'Liberation Sans 12'.)
> 
> Another thing is that you use "settings.get_string" to grab the user font.
> That should give you a string irrespective of whether there actually is a
> specific user value or not. If not it gives you the system default ('Ubuntu
> 11' in standard Ubuntu).
> 
> In the equivalent variant of my code I used "settings.get_user_value" instead,
> which gives you None if there is no specific user value, which means that you
> can reset it with "gsettings reset".
> 
> With your way, the user dconf database will always get a font-name value even
> if there was no such value when the upgrade started. Maybe subtle and not so
> important...
In the general case, get_string('font-name') should give the same as get_user_value('font-name').get_string(), and if not I think getting the system default is the right thing to do.
-- 
https://code.launchpad.net/~enr0n/ubuntu-release-upgrader/+git/ubuntu-release-upgrader/+merge/452304
Your team Ubuntu Core Development Team is subscribed to branch ubuntu-release-upgrader:ubuntu/main.




More information about the Ubuntu-reviews mailing list