Code review comment for ~gunnarhj/ubuntu-release-upgrader:temp-font

Revision history for this message
Gunnar Hjalmarsson (gunnarhj) wrote (last edit ):

At last I think we have a proposal that works as intended.

As regards dconf ignoring the HOME and DBUS_SESSION_BUS_ADDRESS variables it struck me that subprocess spawns new processes and inherits the environment of the current process. So I switched back to using subprocess instead of Gio.Settings, and that confirmed the hypothesis. Now it interacts with the dconf database of the user.

Another observation is that the code in _set_generic_font() is run multiple times for some to me unknown reason. I have added a check to prevent misbehavior due to that. It may not be the most elegant code you can think of...

And @Nick: /tmp is still used for a few things. Some day I may be motivated to learn about Python classes, attributes etc., but I won't struggle with that in a context where testing is so cumbersome as is the case with ubuntu-release-upgrader. So if you (or somebody else) want to polish the code in that respect, please go ahead.

So is the font rendering issue handled now? Almost, but not quite. It changes back to the Ubuntu font in connection with showing the window where the question about rebooting is asked, and then the characters are messed up. But maybe we can live with that. It's still a huge improvement that it happens only so late.

« Back to merge proposal