Improve plugin documentation#1390
Conversation
85fa207 to
49c7365
Compare
49c7365 to
f11db2c
Compare
f11db2c to
aef467c
Compare
| @@ -147,7 +153,14 @@ pyproject.toml | |||
| dependencies = ["bsk"] | |||
There was a problem hiding this comment.
The sample pyproject.toml still declares runtime dependencies = ["bsk"] while the new text says plugin and Basilisk versions must match. Since wheels use [project].dependencies at install time, this can install or accept an incompatible Basilisk runtime. Show bsk==2.X.Y there too, or an explicit compatible version constraint.
| # add its implementation from the SDK: | ||
| # list(APPEND PLUGIN_SOURCES "${BSK_SDK_RUNTIME_MIN_DIR}/atmosphereBase.cpp") | ||
|
|
||
| # An example of this is in the `custom-atm-plugin` example in the SDK repo. |
There was a problem hiding this comment.
The new comment says the custom-atm-plugin example shows appending BSK_SDK_RUNTIME_MIN_DIR/atmosphereBase.cpp, but the actual SDK example says the opposite: bsk_add_swig_module adds SDK runtime sources automatically. This advice can lead plugin authors to duplicate SDK sources or chase a nonexistent requirement.
|
|
||
| [build-system] | ||
| requires = ["scikit-build-core>=0.9"] | ||
| requires = ["scikit-build-core>=0.9.3", "bsk-sdk==2.X.Y", "bsk==2.X.Y"] |
There was a problem hiding this comment.
The new build-system.requires example omits swig==4.4.1, but the SDK CMake macro calls find_package(SWIG REQUIRED) and the working SDK example includes the pip SWIG package. On a clean machine without system SWIG, python -m build --wheel can fail at configure time.
Description
Some of the plugin docs were out of date given the latest
bsk_sdkrelease. This PR updates the CMake code snippets accordingly. Additionally, other minor cleanup is performed to illustrate plugins containing a mix of cpp and python modules.Finally, the diagram was updated to use graphviz.
Drive-by changes
The graphviz package is not mentioned in the macOS install instructions so it was added. Also local documentation builds were not properly pulling in the brew installed graphviz package so
conf.pywas updated to be able to find it.Verification
Built the docs locally and checked it.
Documentation
See above.
Future work
N/A