Skip to content

Conversation

@schaubh
Copy link
Contributor

@schaubh schaubh commented Dec 31, 2025

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

Description

A year ago the BSK module FacetedSRPDynamicEffector underwent a refactor which started to use setter and getters to access module variables. The variables numFacets and numArticulatedFacets were kept public to avoid immediately breaking code. But, their public use was departed. This branch makes these variable private and updates the module documentation.

Verification

BSK built and ran without issues.

Documentation

Documentation built without issues or warnings.

Future work

None

@schaubh schaubh requested a review from leahkiner December 31, 2025 16:35
@schaubh schaubh self-assigned this Dec 31, 2025
@schaubh schaubh requested a review from a team as a code owner December 31, 2025 16:35
@schaubh schaubh added the enhancement New feature or request label Dec 31, 2025
@schaubh schaubh added this to Basilisk Dec 31, 2025
@schaubh schaubh moved this to 👀 In review in Basilisk Dec 31, 2025
This module created setter and getter functions in December 2025 and using numFacets and numArticulatedFacets as public variables has been deprecated for a year now.  This commit makes these two variables now module private variables that must be accessed via the module setter and getter methods.
The module RST documentation no longer needs to warn about the public use of these two variables.
@schaubh schaubh force-pushed the feature/remove_deprecated_items branch from 649a6aa to 1b318ef Compare January 3, 2026 15:41
Copy link
Contributor

@leahkiner leahkiner left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for making this update!

@leahkiner leahkiner merged commit e5280c3 into develop Jan 5, 2026
32 of 34 checks passed
@leahkiner leahkiner deleted the feature/remove_deprecated_items branch January 5, 2026 16:02
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Basilisk Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants