[Sugar-devel] [PATCH sugar-toolkit] presence: use RoomConfig1 to configure channel properties (#3629)

[Daniel Drake]

On Tue, Jul 3, 2012 at 8:04 AM, Sascha Silbe <silbe at activitycentral.com> wrote:
> It seems the if/elif blocks and the else block now handle cases that are
> different in nature. While if block vs. elif block distinguishes between
> the versions of Telepathy in use, if/elif blocks vs. else block
> seems to distinguish between different stages of the protocol. That's
> fine, but a comment explaining the latter check would make this code
> block easier to understand.

I don't think its related to different stages in the protocol, but I
don't know a huge amount on this topic. Can you point me towards what
makes you think this so that I can write a good comment?

>> +    def __update_room_config(self):
>> +        # FIXME: invite-only ought to be set on private activities; but
>> +        # since only the owner can change invite-only, that would break
>> +        # activity scope changes.
>> +        props = {
>> +            # otherwise buddy resolution breaks
>> +            'Anonymous': False,
>> +            # anyone who knows about the channel can join
>> +            'InviteOnly': False,
>> +            # vanish when there are no members
>> +            'Persistent': False,
>> +            # don't appear in server room lists
>> +            'Private': True,
>> +        }
>
> This part seems to be a direct copy of the corresponding block in
> __list_properties_cb(). How about factoring it out into a class
> constant?

It's not a direct copy - the names are different, as are the set of
properties (mentioned in commit message).

I'll address the other comments, thanks.

Daniel