[Sugar-devel] Great Response was Re: [PATCH sugar] Removed hardcoded server url (SL #1976)
dfarning at gmail.com
Wed Oct 13 12:45:00 EDT 2010
On Wed, Oct 13, 2010 at 6:01 AM, Dipankar Patro <dipankar at seeta.in> wrote:
> Thanks for reviewing it, James.
>> if have_ofw_tree():
>> sn = read_ofw('mfg-data/SN')
>> uuid_ = read_ofw('mfg-data/U#')
>> sn = sn or 'SHF00000000'
>> uuid_ = uuid_ or '00000000-0000-0000-0000-000000000000'
>> > else:
>> > sn = generate_serial_number()
>> > uuid_ = str(uuid.uuid1())
>> > - setting_name = '/desktop/sugar/collaboration/jabber_server'
>> > - jabber_server = client.get_string(setting_name)
>> > - store_identifiers(sn, uuid_, jabber_server)
>> > +
>> > + setting_name = '/desktop/sugar/collaboration/jabber_server'
>> > + jabber_server = client.get_string(setting_name)
>> > + store_identifiers(sn, uuid_, jabber_server)
>> > +
>> > + if jabber_server:
>> > url = 'http://' + jabber_server + ':8080/'
>> You are effectively repeating the previous if statement but using the
>> output ... seems a bit obscure.
> ^^ Before applying this : The url (url for registration) was set from gconf
> only for non-XO devices.
> I moved that outside the first if..else you have quoted, so that on all
> devices the url is taken from the gconf property
> the second 'if jabber_server:' is put there to check whether the
> jabber_server retrieved from the gcnof property is empty or unset. If
> jabber_server is a valid one, then change url, other wise keep the url =
> initialized one.
> I actually went with the above modification due to the chain of mails here :
Great response! You clearly explained what you did, _why_ you did it,
and how you came to that decision.
Now a reviewer can effectively make a decision of 'that makes sense,
ACK' or 'what if you look at it this way?'
More information about the Sugar-devel