Wow! Bug 46513 seems like only yesterday. http://cgit.collabora.com/git/user/wjt/telepathy-gabble/log/?h=jingle-rename http://cgit.collabora.com/git/user/wjt/wocky/log/?h=jingle-again I was going to try to preserve history, but I realised that the best way to do it would be to replace the Wocky submodule in Gabble with a git subtree, which seemed like one yak to far, so gave up.
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
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.