Skip to content

Improve plugin documentation#1390

Open
ReeceHumphreys wants to merge 2 commits into
developfrom
fix/plugin_docs_improved
Open

Improve plugin documentation#1390
ReeceHumphreys wants to merge 2 commits into
developfrom
fix/plugin_docs_improved

Conversation

@ReeceHumphreys
Copy link
Copy Markdown
Contributor

@ReeceHumphreys ReeceHumphreys commented May 10, 2026

  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

Some of the plugin docs were out of date given the latest bsk_sdk release. 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.py was updated to be able to find it.

Verification

Built the docs locally and checked it.

Documentation

See above.

Future work

N/A

@ReeceHumphreys ReeceHumphreys self-assigned this May 10, 2026
@ReeceHumphreys ReeceHumphreys added documentation Improvements or additions to documentation enhancement New feature or request labels May 10, 2026
ReeceHumphreys added a commit that referenced this pull request May 10, 2026
@ReeceHumphreys ReeceHumphreys force-pushed the fix/plugin_docs_improved branch from 85fa207 to 49c7365 Compare May 10, 2026 18:01
ReeceHumphreys added a commit that referenced this pull request May 10, 2026
@ReeceHumphreys ReeceHumphreys force-pushed the fix/plugin_docs_improved branch from 49c7365 to f11db2c Compare May 10, 2026 18:06
@ReeceHumphreys ReeceHumphreys force-pushed the fix/plugin_docs_improved branch from f11db2c to aef467c Compare May 10, 2026 18:08
@ReeceHumphreys ReeceHumphreys moved this to 👀 In review in Basilisk May 10, 2026
@ReeceHumphreys ReeceHumphreys marked this pull request as ready for review May 10, 2026 18:09
@ReeceHumphreys ReeceHumphreys requested a review from a team as a code owner May 10, 2026 18:09
@@ -147,7 +153,14 @@ pyproject.toml
dependencies = ["bsk"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

2 participants