Code review comment for lp:~pitti/gtimelog/pygi

Revision history for this message
Marius Gedminas (mgedmin) wrote :

Looks good to me.

At the moment I'm having intermittent connection problems (connection refused on port 22) that prevent me from merging your branch, so I'll do that a bit later. I barely managed to push out a bugfix (unrelated to GUI code) and a small change to the app indicator menu earlier today. Speaking of which, I hope working GtkCheckMenuItems hasn't changed in any significant way between PyGtk and PyGI?

Also, we're hosting a 48-hour Game Jam in our office, so I might be a bit busy.

I think I'd also like try a few more refactorings, e.g. those constructor calls can also be handled the same way as constants:

    try:
        # PyGI
        ...
        new_pango_tab_array = pango.TabArray.new
        ...
    except ImportError:
        # PyGTK
        ...
        new_pango_tab_array = pango.TabArray
        ...
    ...
            tabs = new_pango_tab_array(2, False)
            tabs.set_tab(0, PANGO_TAB_LEFT, 9 * em)
            tabs.set_tab(1, PANGO_TAB_LEFT, 12 * em)
    ...

and constants such as PANGO_TAB_LEFT would look a bit more conventional in upper case. But those are minor details.

review: Approve

« Back to merge proposal