Skip to content

refactor!: remove five unused public surfaces before the 1.0 freeze (1.0.0) - #231

Merged
facontidavide merged 2 commits into
mainfrom
refactor/remove-dead-surface
Nov 13, 2026
Merged

facontidavide merged 2 commits into
mainfrom
refactor/remove-dead-surface

Conversation

@facontidavide

Copy link
Copy Markdown
Contributor

What

Removes five public surfaces that nothing uses, so 1.0.0 does not freeze them until 2.0. PJ_ABI_VERSION stays 5 and no remaining struct, vtable or wire format changes.

Removed Replacement Notes
pj.colormap.v1 (C vtable, ColorMapRegistryView/Service, ToolboxPluginBase::colorMapRegistry()) none No host ever registered it. The callback contract had no lifetime or revocation rule. The id is retired with a comment; a future service would be pj.colormap.v2.
ConfigEnvelope (pj_plugins/host/config_envelope.hpp) none The host never used it. Five doc passages that said it did are corrected.
descriptor_import compatibility surface (four forwarding headers, descriptor_import_support target and Conan component) plotjuggler_sdk::source, pj_base/sdk/source/, PJ::sdk::source Deprecated in 0.31 "for one release". The live pj.descriptor_import.v1 protocol headers are untouched.
pj.parser_functional.v1 pj.parser_functional.v2 Only the v1-only declarations go. The scalar sink and callback types that v2 reuses stay. The id is retired with a comment.
MediaMetadataBuilder (pj_base/sdk/media_metadata.hpp) ObjectTopicMetadataBuilder

Why each is safe

Two independent passes searched PlotJuggler, pj-official-plugins and every sibling repository, including worktrees and unmerged remote branches.

  • colormap: the one plugin that used it, toolbox_colormap, was deleted in August with the Lua/Python toolboxes. PJ4 defines a host for it but never registers it.
  • functional v1: v1 shipped in 0.21.0 on Aug 8 and v2 in 0.22.0 on Aug 9. No published plugin was built in that window, and no host calls either functional path today. A plugin that exposes only v1 is now treated like one that exposes no functional extension.
  • descriptor_import forwarders: no consumer; the only references are in a stale, already-merged worktree.

Test coverage

  • The four descriptor_import_*_test.cpp files are retargeted to source/, not deleted: 56 cases before and after.
  • examples/sdk_consumer/descriptor_import_probe.cpp is renamed source_probe.cpp and retargeted, because it is the only installed-package test that links the compiled pj_source archive.
  • The functional-extension test keeps 23 cases. The v2-to-v1 fallback case is replaced by one proving a v1-only plugin gets no functional route.
  • Removed outright: 3 ConfigEnvelope cases and the 9 MediaMetadataBuilder cases.

PJ4 follow-ups when it adopts 1.0

Both break the PJ4 build until done:

  • Delete pj_datastore/src/colormap_registry_host.cpp, pj_datastore/include/pj_datastore/colormap_registry_host.hpp and the CMake entry. makeColorMapRegistryHost is never called.
  • Remove the unused #include "pj_base/sdk/media_metadata.hpp" in pj_app/tests/main_window_scene3d_drop_framing_test.cpp.

Not in this PR, on purpose

  • pj_emit_plugin_manifest: still called from 33 CMake files in pj-official-plugins, and that repository's floor checker hard-codes the name. It needs a migration there first.
  • PluginDescriptor::min_plotjuggler_version: documented as deprecated, but the Windows installer build and current plugin manifests still rely on the key.

Overlap with #230

Both PRs edit the same lines in pj_base/CMakeLists.txt (install list), test_sdk_install.sh, conanfile.py, cmake/plotjuggler_sdkConfig.cmake.in and two CLAUDE.md files. Whichever merges second needs a small rebase.

Verification

  • ./build.sh --debug (Debug + ASAN): builds
  • ./test.sh: 106 of 106 pass
  • tools/feature_floors/check_feature_floors.py: OK, 138 surfaces
  • ./test_sdk_install.sh (Release): passes
  • pre-commit (clang-format 22): clean
  • conan create (Release, with_tests=False) plus a throwaway consumer that links plugin_host and source and asserts the deleted headers and the descriptor_import_support target are absent: passes

Not run locally: the abidiff gate (libabigail is not installed here). CI runs it. If it flags the removed C types, the baseline refresh is the intentional-MAJOR case.

🤖 Generated with Claude Code

facontidavide and others added 2 commits November 14, 2026 00:37
…1.0.0)

Nothing in PlotJuggler, pj-official-plugins or the sibling repositories uses
any of these, and 1.0 would otherwise keep them until 2.0.

- pj.colormap.v1: the C vtable, ColorMapRegistryView/Service and the
  ToolboxPluginBase accessors. No host ever registered the service, and its
  callback contract had no lifetime or revocation rule. The id is retired;
  a future service would be pj.colormap.v2.
- ConfigEnvelope: the host never used it. Docs that described the host
  wrapping source configs in it now say what the host does.
- The descriptor_import compatibility surface (four forwarding headers, the
  descriptor_import_support target and Conan component), deprecated in 0.31
  for one release. Its tests and the installed-package probe are retargeted to
  pj_base/sdk/source and PJ::sdk::source; no coverage is dropped.
- pj.parser_functional.v1: superseded by v2 one day after it shipped. Only the
  v1-only declarations go; the scalar sink and callback types v2 reuses stay.
  The handle negotiates v2 only. The id is retired.
- MediaMetadataBuilder: superseded by ObjectTopicMetadataBuilder.

PJ_ABI_VERSION stays 5 and no remaining layout changes.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…uses a real table

With the v1 fallback gone, a parser built against 0.21 has no functional route,
so the docs and comments that said "pre-0.21" or "falls back to v1" now say
pre-0.22 and v2 only. The retired-id comment names 1.0.0-dev.1, which still
exported v1.

The v1-only test answered the retired id with a zero-sized table, which a
restored fallback would have rejected on size alone. It now answers with a
well-formed table and asserts neither entry point is called.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@facontidavide
facontidavide force-pushed the refactor/remove-dead-surface branch from f1959d8 to ad2c38c Compare November 13, 2026 23:38
@facontidavide
facontidavide merged commit dee0c09 into main Nov 13, 2026
7 checks passed
@facontidavide
facontidavide deleted the refactor/remove-dead-surface branch November 13, 2026 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant