-
Notifications
You must be signed in to change notification settings - Fork 14
Description
Is your feature request related to a problem? Please describe.
I am building OSTK using Bazel, and pushing a combination of our own code and OSTK code for both C++ and Python use cases. The Python bindings don't quite work for this use case as is, because of the complexities of shared library linking and python paths. I've made it work, but keeping up with version changes can be a bit of a chore (see, for instance, issue 517). We also have long compile times requiring high memory for some of the files.
Describe the solution you'd like
Add one or more of the following style rules to how the pybind files are written:
- Always "Include what you use" -- any header file or using declaration referenced in a .cpp file should also be at the top of that .cpp file. Currently, they rely on include ordering, which is very error-prone.
- Any .hpp file should include a header guard (#ifndef). If that .hpp file appears in multiple OSTK libraries (like ShiftToString.hpp, for instance), it should have the same header guard in every file (ie, "#ifndef UTILITY_SHIFTTOSTRING_HPP").
- Instead of using #include for .cpp files, add them as compilation targets and rely on function forward declarations.
#3 is the most invasive. The other two can be done quite safely, whereas #3 requires changing your BUILD rules (and requires me to change mine too, of course). However, #3 would make it easier to enforce the other ones and would result in faster compile times.
Describe alternatives you've considered
Currently, we are patching these libraries as needed instead, doing the minimal amount of #1 and #2 to get it to work for us.
Additional context
Love the libraries, esp. the new features in Astrodynamics this week!