#8876 NORM 9.1.0: Make sugar control panel support selection of multiple languages

[Zarro Boogs per Child]

#8876: Make sugar control panel support selection of multiple languages
---------------------------------+------------------------------------------
           Reporter:  sayamindu  |       Owner:  marco             
               Type:  defect     |      Status:  new               
           Priority:  normal     |   Milestone:  9.1.0             
          Component:  sugar      |     Version:  Git as of bug date
         Resolution:             |    Keywords:  localization, r+  
        Next_action:  review     |    Verified:  0                 
Deployment_affected:             |   Blockedby:                    
           Blocking:  8875       |  
---------------------------------+------------------------------------------
Changes (by tomeu):

  * keywords:  localization, r? => localization, r+


Comment:

 Replying to [comment:2 sayamindu]:
 > Replying to [comment:1 tomeu]:
 > > {{{
 > > +                lang = locale_code.split('.')[0]
 > > +                lang_column = row[0].split('.')[0]
 > > }}}
 > > May be good to split the ".split('.')[0]" part in a separate function
 named _get_language_code_from_locale(row) so people know what it does.
 > >
 >
 > This is done only once in the code. Don't you think a method would be a
 overkill ? I put in a comment.

 Well, changing a block into a separate method can be very cheap in python,
 compared to most other languages. Not much more expensive linecount-wise
 as adding a comment.

 The problem with having inline comments about how we do things is that we
 need to maintain them when the code changes and in many cases aren't
 updated and thus they become lies. If we change the code structure to
 reflect what it does, we get support from the language (and tools) in
 order to maintain identifiers in sync with the rest of the code.

 I understand that this is the spirit of the guideline I posted above from
 the gnome coding guidelines:

 {{{
 Do not say how it does it unless it is absolutely necessary; this should
 be obvious from reading the code. If it is not, you may want to rework it
 until the code is easy to understand.
 }}}


 I pointed out in the review some inline comments that I thought should be
 removed and you indeed removed those, but not others. IMO, we should
 strive to use identifiers good enough so that don't need comments to be
 sprinkled around in the code.

 {{{
         self._stores = []           # List to store liststores
 }}}

 If you feel the need to add such a comment, then your code might not be
 structured clear enough. I would probably name that var _list_stores
 instead and remove the comment that would add exactly zero substance.

 I'm r+ anyway, just wanted to make clear my opinion on this subject.

-- 
Ticket URL: <http://dev.laptop.org/ticket/8876#comment:3>
One Laptop Per Child <http://laptop.org/>
OLPC bug tracking system