Revision f6a67901e79d8d35e6bf30f0766b2417740090b7

[Andreas Monitzer]

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  
> should
> 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  
> send
> presence until we receive the roster.  If we don't wait for the  
> roster,
> we get presence we're not expecting, and no one shows up the first  
> time
> 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  
> that's
> not his code, but I made the same comment about the original patch  
> when
> 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?

andy