On Thu, May 11, 2023 at 10:42 AM Calvin Wan <calvinwan@xxxxxxxxxx> wrote: > > > This series continues to focus on splitting declarations from cache.h to > > separate headers, and also cleans up some other small header issues. By > > patch 16, cache.h is gone. > > Congratulations on slaying the cache.h beast! Glad I was able to follow > the entire series to see how many headers cache.h eventually splits up > into. Thanks. :-) > Most of the patches in this series LGTM, but I do have a higher level > concern. A couple of patches in this series splits files from foo.h to > foo-ll.h and foo.h with the goal of having other files only include > foo-ll.h when needed and not foo.h which has other unnecessary includes. > While I believe that having less unnecessary includes clarifies which > files have what dependencies, I also believe that we are missing > documentation for how new functions should be added to either foo.h or > foo-ll.h. What criteria are we using to separate out those functions? Right now it's a bit loose, and focused around structs rather than functions and function declarations. I'd state the guidelines roughly as: * The foo-ll.h variants should only #include whatever headers are necessary for the common structs they contain to be well-defined * The foo.h variants should take things (particularly inline functions) that require additional includes * We should prefer forward declaring structs rather than #include'ing a header where we can (though that's a bunch of patches in a follow-on series I've prepared) This is even a bit murky because of my use of the word "common". For example, I kept odb_path_map out of object-store-ll.h and left it in object-store.h, because there were only 2 files that used struct odb_path_map, while there are 56 that use the structs in object-store-ll.h, and the declaration of odb_path_map requires multiple extra headers. Now, you specifically asked about where function declarations and functions should go, and I side-stepped that question. In part, I'm leaving that undefined precisely because I was focused only on header cleanups and not source cleanups, and I suspect that diving into the code will yield insights about where we want to fit the boundary for those. So, for the many function declarations that could go in either header and satisfy all the above rules, I think the libification work you and Glen are doing is likely to nail this down a bit more. Once we have rules for how to divide those up, or even just a few examples that we have good arguments for, then we can happily start moving the function declarations between these headers. Does that sound fair?
