Bug 58198

This was actually a lot smaller than I was expecting.

The gabble branch is obviously fine. I seriously lolled at your perl script though. u r such a l33t haxxor despite what you try to tell us all.

Yeah should the quirks really be in the public API? (yes I realise talking about public API with Wocky is a bit of a joke) They could at least be more internal, no?

I didn't check all the new jingle docstrings word for word; I reckon they'll be fine.

I see you've moved this stuff into wocky/jingle-*.[ch]. Although I hate the "wocky-" prefix as much as anyone else, I think inconsistency is even worse. What are your thoughts on this? Perhaps it's less of a problem given we have a big wocky.h now? Speaking of which...

The new headers don't have the WOCKY_H_INSIDE check.

I did a git grep -i gabble in your wocky tree and there are loads of references that probably shouldn't be around anymore, like header guards, comments listing the filename as gabble-jingle-session.c and stuff like that. It would be nicer to have less of this.

Otherwise hot. I wonder if we could feasibly have roughly the same test coverage of the Jingle code directly in Wocky one day? Sounds pretty tricky to me tbh.

(In reply to comment #1)
> Yeah should the quirks really be in the public API? (yes I realise talking
> about public API with Wocky is a bit of a joke) They could at least be more
> internal, no?

How could they be made “more internal”? The JingleSession & friends needs to know stuff, and Gabble needs to answer the query-caps signal.

Perhaps one solution would be to remove the need for some of the quirks. Another solution would be to move more of the caps machinery down.

> I didn't check all the new jingle docstrings word for word; I reckon they'll
> be fine.

There'll be more where they came from.

> I see you've moved this stuff into wocky/jingle-*.[ch]. Although I hate the
> "wocky-" prefix as much as anyone else, I think inconsistency is even worse.
> What are your thoughts on this? Perhaps it's less of a problem given we have
> a big wocky.h now? Speaking of which...

Yeah, I think it's probably worse to be inconsistent.

> The new headers don't have the WOCKY_H_INSIDE check.

Nor are they included in wocky.h, so that's okay ;)

> I did a git grep -i gabble in your wocky tree and there are loads of
> references that probably shouldn't be around anymore, like header guards,
> comments listing the filename as gabble-jingle-session.c and stuff like
> that. It would be nicer to have less of this.

Yes there are, I'll fix this.

> Otherwise hot. I wonder if we could feasibly have roughly the same test
> coverage of the Jingle code directly in Wocky one day? Sounds pretty tricky
> to me tbh.

You're not wrong!

I think you reviewed up to 82f2006 in Wocky. I have pushed 12 more patches which add some more docs and then fix your complaints here.

I have also pushed two more patches up to Gabble which update it for the single-include stuff.

Your new patches look fine. Again I didn't really review the docstrings word for word, but your reputation precedes you.

Hot.

(In reply to comment #4)
> Your new patches look fine. Again I didn't really review the docstrings word
> for word, but your reputation precedes you.

I think that you mean that my reputation *proceeds* me.

Merged, and NEWS updated for 0.17.3. http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=0d65aee3