pidgin at monitzer.com
Tue Aug 28 02:51:48 EDT 2007
First off, thank you for reviewing the code.
On Aug 28, 2007, at 03:28, Nathan Walp wrote:
> - I dislike implementing experimental JEPs in libpurple
The problem is that the XSF is very slow in moving XEPs to more
stable states, it usually takes a few years. I asked people in the
know about all experimental XEPs I considered implementing before
doing so. All of them are not expected to change much or getting
thrown out again. That's the reason I didn't implement User Profile.
XEP-0115: Entity Capabilities has changed since I implemented it, the
implementation will need some modifications.
> - forces buddy icons to PNGs, which will kill animated buddy icons
XEP-0084 says that PNG is required. It's possible to specify multiple
alternatives (at least one of them has to be PNG), but libpurple's
API doesn't allow for that, so I had to force it to PNG only.
> - the prpl now knows way too much about PNGs. That functionality
> be abstracted out to somewhere else, core or otherwise.
Yes, knowing the image size would be great. I hope you understand
that I hesitated rewriting the core API for my project.
> - turns minor get-info code duplication in buddy.c into massive code
> duplication. Abstract that out to a function or something.
Should be easy to fix.
> - for the gateway interaction stuff, we assume that anything in our
> roster w/o an '@' is a gateway. Why don't we simply send a disco
> request to all such entries in the roster, so we can know for sure?
You wouldn't be any wiser for nodes that are offline (even gateways
can be offline). I talked to stpeter about that at last year's SoC,
there simply isn't a proper solution for it.
Besides that, doing a disco#info storm is one of the main things
XEP-0115 is trying to avoid. That XEP only works when you have
received a presence packet from the peer, which is not the case when
the gateway is offline, but it is needed to recognize it as a
gateway, so you can tell it that you want to go online.
> - reverts a fix in jabber_disco_finish_server_info_result_cb to not
> presence until we receive the roster. If we don't wait for the
> we get presence we're not expecting, and no one shows up the first
> you log in.
This looks like a simple mtn merge error to me.
> - ping.[ch] seems like overkill for such a small feature (I know
> not his code, but I made the same comment about the original patch
> it was submitted in the tracker)
I don't quite see the cost in having those two files there.
> - inconsistent indenting throughout
Could you give an example?
More information about the Devel