Ticket #24 (new enhancement)
muc client support
| Reported by: | tofu | Owned by: | tofu |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | generic | Keywords: | |
| Cc: | ralphm | Author: | |
| Branch: |
Description
MUC client protocol support is needed for creating generic xmpp clients that support multi-user chat.
Change History
comment:7 Changed 23 months ago by tofu
I believe I have the basics ready for review. By basics I support the basics defined in the XEP. I also support each part of the Use cases, Members, Moderators and Owners. I have some tests for each of these cases. I feel we should try and get what I have merged and then for additional features we will make separate tickets.
One ticket would eventually have an API similar to the regular chat API in twisted.words.
Please review.
comment:8 Changed 21 months ago by ralphm
- Status changed from assigned to new
This is my first review of this code, mostly based on coding style, docstrings, interface consistency and tests. Please don't be discouraged by the strictness, but it is required if we want to move this to Twisted and has proved to make better code in the end. Let's figure out how we can address these points in several steps.
In general, I'd like Wokkel to adhere to the Twisted Coding Standard which in turn depends on Python's Style Guide. Futhermore, the expectations in terms of testing are mentioned in Unit Tests in Twisted. More specific comments follow:
The protocol additions for Service Discovery should be done in #28, so that makes this branch depend on that. I have some thoughts about how that should be done, but we can solve that in some future iteration.
On IMUCClient:
- Every interface method must have docstrings that explain what it is for and what its parameters are, what they are for and what their type(s) may be. Also document what is returned by a method and the return type.
- Parameters, like other variables, must be in camelCase and have descriptive names. E.g. roomJID instead of room_jid. Also, to doesn't tell me much in itself, so maybe recipient or occupant would be better.
- Try to maintain a consistent interface. The join method has server and room arguments, whereas leave has room_jid. And then userJoinedRoom has room, which is a muc.Room instance.
- Don't use empty lists as default values for function/method arguments:
>>> def a(b=[]): ... b.append(3) ... print b >>> a() [3] >>> a() [3, 3] >>> a() [3, 3, 3]
In muc.py:
- The naming of classes here should use some additional context. Prefixing MUC would be nice.
- I am a bit mystified about sendDeferred being a public method in IRCClient. This looks similar to the IQ reply functionality in Words, and could some more generalization.
Well, that's enough for this round.
comment:10 Changed 13 months ago by kapilt
i'd be happy to help clean it up, the review items look pretty minor.
comment:11 Changed 13 months ago by ralphm
I've been working on this, but it is a bit stalled at the moment. My current work is here: http://hg.ik.nu/ralphm/wokkel-muc-client-support-24-2. I hope to fix it up soon.
