Includes Reorganization #788
Hoikas
started this conversation in
Ideas & Proposals
Replies: 1 comment
-
After some discussion on IRC related to the source tree structure, I'm changing my recommended header organization to look like this:
This should make navigating to a header file require less directory backtracking. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
While working on #383 to make our cmake more target-based and cleanup the dependency linkage, I've found lots of issues with our includes throughout the codebase. I have most commonly encountered the following issues:
The results of these issues are longer compile times,
#include
side-effects, and difficulty in understanding a target's dependencies. Among the side effects, some headers seem to (still) bring inwindows.h
, which defines many things that conflict with Plasma's code (egDrawText
andGetObject
) -- this can break in subtle ways if includes are reorganized.With that in mind, I would like to propose the following rules for includes:
Headers should not define the body of virtual functions.hsWindows.h
orwindows.h
Pch.h
(dedicated precompiled header)Includes should be arranged in blocks ordered as demonstrated:
Or, in text:
Within each block, all includes should be alphabetized without considering any prefixes. This will enable casual perusal. Any 3rd party or STL includes may form additional blocks following these rules. These blocks must come after the cpp file's include.
Further, when #383 is complete, I believe we should reorganize all headers into the following structure:
This will ensure that any target that wants to consume
pfPatcher
must link againstpfPatcher
to even see its headers. It would continue to allow targets to include their own headers directly by name and for other targets to include things by<target>/<file>
. Further, it allows for stronger (but not maximum) enforcement of what is a private header... For example, only theplPhysX
target should be including the fileplPhysXAPI.h
, which includes actual PhysX headers.Beta Was this translation helpful? Give feedback.
All reactions