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

[SugarLabs Bugs]

#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:

 Thanks, Hamilton, some more comments follow:

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

 No need to import os.path. Also see in
 http://www.python.org/dev/peps/pep-0008/ how imports are to be grouped.

 {{{
 +def get_epoch():
 }}}

 Which epoch is that? Seems to me that what you want is the current time
 expressed in the number of seconds from the Unix epoch? If that's the
 case, time.time() should be enough.

 {{{
 +    t = datetime.datetime.now()
 +    e = time.mktime(t.timetuple())
 }}}

 Please use variable names that describe what the variables are for. This
 also applies to other parts of the patch.

 {{{
 +def create_identifier_serial():
 }}}

 What this function does? Maybe we can find a better name.

 {{{
 +def create_identifier_uuid():
 +    uuid_string = uuid.uuid1()
 +    uuid_string = str(uuid_string)
 +    return uuid_string
 }}}

 Not sure it's worth a function for something that can be expressed clearly
 with str(uuid.uuid1()).

 {{{
 +def write_identifier_info(sn,uuid,backup_url):
 }}}

 Please see in http://www.python.org/dev/peps/pep-0008/ about whitespace.
 This also applies to other parts of the patch.

 Also, please put an empty line between the different parts of this
 function (maybe where you had comments before).

 {{{
 +    identifier_dir = os.path.expanduser('~') +
 '/.sugar/default/identifiers'
 }}}

 Please see this recent thread about this:
 http://lists.sugarlabs.org/archive/sugar-devel/2009-August/017887.html

 {{{
 +        JABBER_SERVER =
 client.get_string('/desktop/sugar/collaboration/jabber_server')
 }}}

 We use all caps for static constants, dynamic constants are named as
 normal variables.

 {{{
 -    except (Error, socket.error):
 -        logging.exception('Registration: cannot connect to server')
 +    except (Error, socket.error), e:
 +        logging.error('Registration: cannot connect to server: %s' % e)
 }}}

 Why doing this?

 {{{
 -        logging.error('Registration: server could not complete request:
 %s',
 +        logging.error('Registration: server could not complete request:
 %s' %
 }}}

 And this?

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