Code review comment for lp:~jtatum/mago/gcalctool

Revision history for this message
Ara Pulido (ara) wrote :

Hello James,

Excellent addition. I specially liked the way you solved the question of menus and buttons using lists.

Only a couple of comments:

In gcalctool_views.py and gcalctool_calculations.py you seemed to haved copypasted the skeleton from gedit_chains.py, and added an import (time, sfrtime...) not needed in these cases (minor issue, really).

Also, in gcalctool_views suite, you just check that the value has not changed. I don't think this is the correct thing to check. Indeed, it is also good to check that, it can be kept, but I would have also had checked things like:

* Buttons that only exist for that view, are now visible
* Window name has changed

Just some possible improvements to an overall very good job.

review: Needs Fixing

« Back to merge proposal