[Sugar-devel] [PATCH] Remove hard-coded School Server URL when XOs register #sl1976

[Tomeu Vizoso]

On Sun, Jul 4, 2010 at 11:42, Sascha Silbe
<sascha-ml-ui-sugar-devel at silbe.org> wrote:
> Excerpts from Tim McNamara's message of Sat Jul 03 22:33:50 +0000 2010:
>
>> Ref http://bugs.sugarlabs.org/ticket/1976
> Thanks for working on this!
>
>> Bevaviour of the software is now independent of the hardware that Sugar is
>> running. All registrations will look for Jabber server settings from
>> /desktop/sugar/collaboration/jabber_server. However, this patch retains the
>> REGISTER_URL as an option for systems administrators. This prevents
>> currently written documentation for the XS / XO registration from breaking.
> Administrators should not need to change the Sugar source code. For one
> thing it will interact badly with package management systems.
> Let's rather adjust the documentation.

Agreed.

>> Please note, I haven't got an XS to test this on, so am relying on
>> maintainter to see if this fixes the bug.
> We're working on setting up a public XS instance for testing, but there are
> security concerns. In the meantime you might try installing XS in a local
> VM.

Also, as long as we have a shortage of maintainers, it's better to try
to offload as much as possible from them. Asking in the mailing list
for help testing is fine.

>> +    setting_name = '/desktop/sugar/collaboration/jabber_server'
>> +    jabber_server = client.get_string(setting_name)
>> +    store_identifiers(sn, uuid_, jabber_server)
> While you're at it, can you rename uuid_ to e.g. our_uuid, please?
> A trailing underscore means "we don't use it" (but got it for one
> reason or another, e.g. because the API of some other component
> returned more information than we need).

http://www.python.org/dev/peps/pep-0008/ mentions trailing underscore
to avoid a conflict with a Python keyword. "we don't use it" derives
from our usage of pylint. In this case it's a module name and I
personally don't care between our_uuid or uuid_ but please try to
avoid using a less clear name just to avoid a name clash.

>> +    if len(url) == 0:
>>          url = 'http://' + jabber_server + ':8080/'
> The if won't be needed once you drop REGISTER_URL, but in general
> I'd suggest to use "not url" instead as it will catch both an empty
> value and None. Depending on what exactly you're testing against
> there might even be a performance benefit, but it doesn't matter
> for strings, lists or dictionaries (at least with CPython).

"not url" is what is recommended in PEP-8.

Thanks for the patch and the review,

Tomeu

>> +    else:
>> +        url = url
> Superfluous even before dropping REGISTER_URL.
>
> Sascha
>
> --
> http://sascha.silbe.org/
> http://www.infra-silbe.de/
>
> _______________________________________________
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel
>
>