Skip to content

chore!: namespace public headers and prepare v3.0.0 - #27

Merged
SebastianBoehler merged 12 commits into
mainfrom
chore/agent-guidance-release
Oct 5, 2026
Merged

SebastianBoehler merged 12 commits into
mainfrom
chore/agent-guidance-release

Conversation

@SebastianBoehler

@SebastianBoehler SebastianBoehler commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

The repo now gives coding agents and contributors a clear workflow: small modules, efficient code, clear variable names, shared helpers, scoped commits, and evidence-backed checks. It captures the lessons from the user stream and position operations reviews.

Pinned clang-format, clang-tidy, and Prettier checks run in CI. They target changed C++ lines and new formatting issues, so existing code does not need a broad cleanup. Linux Release checks the PR title, formatting, and lint against its existing build and installed consumer. This removes the separate quality build and reduces the workflow from six jobs to four. PR titles use Conventional Commits. Editor settings match the C++ style, builds avoid duplicate branch runs, and compiler parallelism is bounded on hosted runners.

This prepares v3.0.0 for the merged user stream and position operations. It also fixes the public header collision reported by Bill: all SDK headers live under include/polymarket/, and consumers use includes such as <polymarket/http_client.hpp>. There are no flat forwarding headers. This requires a major release and a consumer rebuild; the CMake target remains polymarket::client. Versions, installation examples, and curated release notes agree.

The package test rejects flat installed SDK headers, checks every public header is present, and builds against the namespaced SDK alongside a consumer-owned http_client.hpp. The quality script handles renames without forcing unrelated formatting changes.

Validation: local Release build with examples, tests, and benchmarks; the full offline suite and installed-package consumer; formatting and lint; focused quality script tests; rejection of injected naming and formatting violations. The original header collision was reproduced in an external consumer. A fresh targeted build verified the complete installation dependency graph. No funded transactions were run. Linux/macOS Debug and Release CI must pass before publication.

@yluoc

yluoc commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@SebastianBoehler
we have a small portability issue over there, we can apply it if available before releasing a new version.

Header portability: where the problem is and how to fix it

Where: CMakeLists.txt:224
install(DIRECTORY include/ DESTINATION include)
Our 20 public headers sit flat in include/. So make install copies them straight into the system include directory:
/usr/local/include/types.hpp
/usr/local/include/http_client.hpp
/usr/local/include/orderbook.hpp
/usr/local/include/websocket_client.hpp
...

Why it's a problem:

  • These names are generic. Another library, or the user's own project, can easily have its own types.hpp or http_client.hpp.
  • Whichever directory comes first on the include path wins. Users then get confusing compile errors, or worse, code that silently builds against the wrong declarations.
  • The only header we already protect this way is version.hpp, which lives at include/polymarket/version.hpp.

What isn't the problem: #pragma once. The SDK requires C++20, and every compiler that can build C++20 (GCC, Clang, Apple Clang, MSVC, Intel, NVCC) supports #pragma once. Switching to #ifndef guards wouldn't make the SDK buildable anywhere new. It would also add a risk: a guard like TYPES_HPP can collide with someone else's, and a header with a colliding guard silently compiles to nothing.

The fix: move all public headers to include/polymarket/ and include them with the prefix:
#include "polymarket/clob_client.hpp" // instead of "clob_client.hpp"
After installing, everything lands in /usr/local/include/polymarket/, so it can't collide with anything. The install rule doesn't need to change.

Status: I prototyped the move: 141 files, with only include lines changed. The build passes and all 48 tests pass, including the test that installs the SDK and compiles a separate project against it. The work is saved in git stash (stash@{0}) and not committed.

What we need to decide: this breaks every user's include paths. The options are:

  1. A hard break in a major release (3.0.0, up from the current 2.0.0) with a release note.
  2. Keep small forwarding headers at the old paths for one release. They would include the new path and print a deprecation warning, and we'd remove them in the next major release.

BREAKING CHANGE: public SDK includes now require polymarket/. Rebuild consumers against the v3.0.0 headers and libraries.
@SebastianBoehler SebastianBoehler changed the title chore: add agent quality checks and prepare v2.1.0 chore!: namespace public headers and prepare v3.0.0 Oct 5, 2026
@SebastianBoehler
SebastianBoehler merged commit e3a193c into main Oct 5, 2026
7 of 8 checks passed
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.

2 participants