Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: HippoDraw - Interactive and Python scriptable data analysis application https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208034 ------- Additional Comments From j.w.r.degoede@xxxxxx 2007-04-08 04:30 EST ------- About the header files, #ifdef HAVE_XXXX and including config.h Header files document / publish interfaces. The first thing todo is to make a divide between internal interfaces (iow interfaces between parts of HippoDraw) and external interfaces. Then you should only install external interfaces, iow interfaces meant to be used by other party apps / libraries into /usr/include. Then where-ever possible the header files installed into /usr/include should not contain any #ifdef HAVE_XXX code, if they do, maybe the contain a mix between internal and external interfaces. If that is the case try to split the header file. Having #ifdef HAVE_XXXX macros in public header files is bad, as that means that different compilations of the same version of HippoDraw may be ABI incompatible, which should be avoided if at all possible. Last when the number of #ifdef HAVE_XXX lines in public header files is reduced the a minimum, change them to #ifdef HIPPODRAW_WITH_XXX, and create a hippodraw_conf.h with only the #ifdef HIPPODRAW_WITH_XXX defines in it, and include that from the header files who need the #ifdef HIPPODRAW_WITH_XXX defines. This last step is necessary because we don't want a generic filename like config.h in the headerfile-name space, nor a header file with very generic and often conflicting defines like HAVE_XXX. What happens when an app uses 2 libraries with config.h, which config.h gets included? And if they both get included, because not -Ixxx is used, but #include <xxx/config.h>, then what if one defines HAVE_FOO 0 and the other as HAVE_FOO 1, as they were build with different BR's? To avoid such a mess in general autoheader files should not be installed, and thus public headers should not depend on them. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review