[sugar] frame redesign

[Simon Schampijer]

Tomeu Vizoso wrote:
> On Wed, Mar 19, 2008 at 5:05 PM, Simon Schampijer <simon at schampijer.de> wrote:
>> Hi Tomeu, Eben,
>>
>>  I went through the last commits until 'Remove activities.defaults, not
>>  used any more'. I will go through the rest as well. Here are my thoughts:
>>
>>  *** Pulse activity icons in the ring during launch
>>  commit 3670fda59103c32fcbe8b28a9f8e77a75ac15d31
>>
>>  pylint:
>>   - src/model/homemodel.py: Unused import dbus (maybe you can remove
>>  that when you fix something in that file)
> 
> Done
> 
>>   - 139: Line too long (81/80)
>>   - 198: Line too long (81/80)
> 
> Do we care about one char more?

If we do it we should do it right i guess :)

>>  _get_color() could be a function since it does not access the actual
>>  object. Probably ok in this case.
> 
> I like to group code in classes when it makes sense only in that
> class. Everybody agree? A later commit makes the method use
> self._level, though.

Yup it makes sense to group that there, having in the scope of the 
module would be wrong as well.

>>  W:211:ActivityIcon: Use of "property" on an old style class
>>  W:215:ActivityIcon: Use of "property" on an old style class
> 
> False positive from pylint, ActivityIcon is a new style class as
> inherits from CanvasIcon.

hmm, again.

>>  Why do we 'wrap' the get_version method here with the property construct?
> 
> Because the code is shorter and leaves more room for in the future
> changing the implementation without breaking API compatibility. I
> think we want to do this in general? Not expose setters nor getters
> but properties?

Agreed as well understood this better when we discussed the custom 
setter getter case today.

>>  *** Made background colors of zoom spheres white, for consistency
>>  commit c5b776bcf27b4f6e638fac3658437a3c194e7ddd
>>   - src/view/BuddyIcon.py
>>       - no semicolon
> 
> Done
> 
>>  *** Fixed scale and placement of a single icon in the ring
>>  commit ef75cabea665c436fc40cee887462931462d6bbf
>>   - what do the '**' instead of '*' mean ?
> 
> It dereferences twice. It's some pygtk magic for getting the rest of
> the named args as a dict.

Oh ok, makes sense.

>>  *** Store favorite activities in the profile dir.
>>  commit 3b5131e4b16eb5a83278de3787a5b1f4763ae566
>>   - no need to import os.path as well we import os already
> 
> Done
> 
>>  *** General Errors
>>  src.view.home.activitiesring:
>>  E: 75:ActivitiesRing.__activity_added_cb: Undefined variable 'info'
>>      - should probably be activity_info
> 
> Done
> 
>>  src.view.home.activitieslist
>>  W: 20: Unused import gtk
> 
> Done
> 
>>  * General Thoughts
>>  What is the advantage from simplejson over json. The home page says:
>>  simplejson is API compatible with json-py, but is more easily
>>  extensible, has better unicode support, is probably also faster, and is
>>  MIT licensed (json-py is LGPL/GPL). Should we recommend people using
>>  simplejson?
> 
> My main problem with json is that it requires the data types to be the
> base types in python. So if you want to encode data from dbus, you'll
> have to convert all dbus.String to str. So perhaps we should go always
> with simplejson?

Makes sense to me.

Simon