GNOME Bugzilla – Bug 408006
Remove gossip-types.h and gossip.h
Last modified: 2007-02-22 17:47:47 UTC
There were discussion about removing gossip-types.h now that Martyn fixed circular deps. The problem is .h files in libgossip doesn't include all deps anymore so each .c file in libgossip, ui and backend should include themself all dependencies in the right order which is a big work to do. The other solution is to simply include gossip.h everywhere.
Created attachment 82564 [details] [review] proposed patch This patch uses gossip.h everywhere and removes gossip-types.h
Xavier, thanks, this is a good start to help me revert the changes I made. Unfortunately :) we won't be using a gossip.h file to be included from everywhere else - although I have no issues with it, Micke would rather it was the way it was I think. Micke, unless you have changed your mind? :)
I would indeed. :)
Created attachment 82591 [details] a script to generate includes of libgossip headers It's not yet finished but should help.
Created attachment 82690 [details] [review] proposed patch Here is a patch to remove gossip-types.h and gossip.h. It's mainly done by regenerating the include list using a script I made.
Created attachment 82691 [details] [review] the script I used For the record I attach here the script used.
Hi Xavier, thanks for making a start on this patch! One problem I have seen is, that there is a lot of duplication. For example, if you take libgossip/gossip-session.[ch], there are a lot of includes in both the source and header file which could be cut down if we just included the header file from the source file (which we do anyway). Now if you consider that recursively, the number of includes you need through out all the .[ch] files would probably be a lot less than it currently is. If you could, it would be better to basically work out what the header file needs in your patch, then include that first in the source file and any other dependencies can then be included after that but nothing that is already in the header file.
I though the goal of not using gossip.h was to easily see on which modules we depend on. If I don't include modules that are recursively included in headers it will be harder to see exactly deps of a module, I think. Avoiding that can only be done manually, scripts can't help here (or far more difficult) :-(
Created attachment 83068 [details] [review] yet another proposal Removed dups. Seems OK ?
Created attachment 83096 [details] [review] Small improvement to Xavier's patch Xavier, the patch looks good, I have updated it slightly to change the TELEPATHY definition to "HAVE_TELEPATHY" and removed the comments in the libgossip/gossip-protocol.c file about needing something in config.h to know if we should include Telepathy files or not. You can commit this, it all looks good to me.
committed, thanks.