[Bugs] #9 MAJO: fix network frame device

[SugarLabs Bugs]

#9: fix network frame device
---------------------+------------------------------------------------------
  Reporter:  erikos  |       Owner:  erikos
      Type:  defect  |      Status:  closed
  Priority:  major   |   Component:  sugar 
Resolution:  fixed   |    Keywords:  r+    
---------------------+------------------------------------------------------
Changes (by erikos):

  * status:  new => closed
  * resolution:  => fixed


Comment:

 Replying to [comment:2 tomeu]:
 > {{{
 > def freq_to_channel(freq):
 > }}}
 >
 > Why not having a global dict called frequency_to_channel instead? Not
 sure it's much better, just a suggestion.
 >
 > {{{
 >         self._chan_label = gtk.Label()
 > }}}

 > Any good reason for the abbreviation? Cannot we have _channel_label
 instead? If we could improve all the chan and freq variables, I think it
 could help quite a bit later to casual readers/bugfixers.

 done.

 > {{{
 > def connecting
 > ...
 > def connected
 > ...
 > def disconnected
 > }}}
 >
 > What about set_connecting_state(), set_connected_state(), etc? or a
 single set_state() receiving a state constant? method names use to have a
 verb in imperative form, which should convey what will happen when it gets
 executed.

 i made it setters, makes sense.

 > {{{
 >     def _disconnect_activate_cb(self, menuitem):
 > }}}
 >
 > should we have one more underscore there?

 Yup.

 > {{{
 >         self._palette = WirelessPalette(self._name, self)
 > }}}
 >
 > Could we avoid somehow this circular reference? Maybe by the palette
 firing a deactivate-clicked signal?

 Made it a signal - thanks very much for that tip.

 > {{{
 > self._active_ap_op
 > }}}
 >
 > Couldn't manage to discover what "op" means here.

 'op' stands for object_path. I thought this might be too long to make it
 self._active_ap_object_path. At least i was consistent on the patch with
 that usage.

 > No more nipicks, but if we could avoid having so many gratuitous
 abbreviations that don't bring any value, I think it would be great. When
 fixing a bug or reading a stack trace, it's better to not have to read the
 whole module in order to understand what's going on.

 If we want to get rid of the abbreviations we should consider adjusting
 the limit of 80 chars as well. i guess 10 more chars could help there
 already. In general i will try to avoid abbreviations.

 Thanks for the detailed review.

-- 
Ticket URL: <http://dev.sugarlabs.org/ticket/9#comment:3>
SugarLabs <http://dev.sugarlabs.org>
Sugar Labs bug tracking system