[Bugs] #916 NORM: Allow sugar on non-XO hardware to register with an XS server

SugarLabs Bugs bugtracker-noreply at sugarlabs.org
Mon Aug 24 05:38:57 EDT 2009


#916: Allow sugar on non-XO hardware to register with an XS server
-----------------------------+----------------------------------------------
    Reporter:  hamiltonchua  |          Owner:  tomeu                      
        Type:  enhancement   |         Status:  new                        
    Priority:  Normal        |      Milestone:  Unspecified by Release Team
   Component:  sugar         |        Version:  Unspecified                
    Severity:  Unspecified   |     Resolution:                             
    Keywords:  r-            |   Distribution:  Unspecified                
Status_field:  Unconfirmed   |  
-----------------------------+----------------------------------------------
Changes (by tomeu):

  * keywords:  r? => r-


Comment:

 {{{
 +import os,os.path
 }}}

 missing space

 {{{
 +# defs to enable registration of sugar to an xs on non-olpc hardware
 +
 +# utilities
 ********************************************************************
 }}}

 Please use API docs instead of inline comments. Explain what the code
 does, not how it does it. This applies to the whole patch.

 {{{
 +def getEpoch():
 }}}

 Don't use camel case, see how the rest of the code is written and try to
 do the same.

 {{{
 +# - we randomly get 3 letters
 +# - concat the above with the last 8 numbers from epoch seconds
 }}}

 If you need to explain what the code does, it's because you can make the
 code clearer:

 {{{
 +  s1 = ''.join([random.choice(string.ascii_uppercase) for y in range(3)])
 +  s2 = str(int(getEpoch()))[-8:]
 +  serial = s1 + s2
 +  return serial
 }}}

 No need to make the code compact and hard to read, put a single expression
 in each line and use good names for each intermediate variable. This
 applies to the whole patch.

 {{{
 +def gen_identifier_serial():
 }}}

 Don't use abbreviations if at all possible. This applies to the whole
 patch.

 {{{
 +  u1_count = 40
 }}}

 What's this magical value? May be better to have it as a constant at the
 module level, the name in all caps and with a leading underscore.

 {{{
 +  uuid = ''.join([random.choice(string.hexdigits + '-') for y in
 range(int(u1_count))])
 }}}

 Split this in several lines, please. This applies to the whole patch.

 {{{
 +        # logging.error('Registration: Cannot obtain data needed to
 register.')
 +        # raise RegisterError(_('Cannot obtain data needed for
 registration.'))
 }}}

 Why this commented out code? If the code is not needed any more, please
 remove.

 When you have a new patch, please do attach a single one that applies to
 the current git HEAD.

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


More information about the Bugs mailing list