|
| 1 | +# CLAUDE.md |
| 2 | + |
| 3 | +LaunchDarkly C++ SDKs monorepo: client SDK, server SDK, server SDK integrations (Redis, OTel), plus shared libraries. See `README.md` for the build/install reference and `CONTRIBUTING.md` for the style basics (Google C++ style, C++17, C99). |
| 4 | + |
| 5 | +## Build & test operational notes |
| 6 | + |
| 7 | +Full reference: `README.md`. A few things that aren't obvious from it: |
| 8 | + |
| 9 | +- **`cmake -B build -S .`** — always pass an out-of-source build directory with `-B`. Don't run cmake with `.` as the build directory; it doesn't work cleanly here. |
| 10 | +- **Multiple build configurations can coexist as sibling build directories**, e.g. one with sanitizers on, one with `LD_CURL_NETWORKING=ON`. A common local convention is `build-nosan/` and `build-curl/`, but don't assume those directories exist — list the directory first before using one. |
| 11 | +- **If test binaries hang at startup with no output**, that's an intermittent ASAN issue seen on some platforms. Try a no-sanitizer build (`-DLD_TESTING_SANITIZERS=OFF`) to confirm. Not universal — don't assume it's always the cause. |
| 12 | +- **When SSE or HTTP networking code changes**, validate against both backends: Boost.Beast/Foxy (default) and CURL (`-DLD_CURL_NETWORKING=ON`). |
| 13 | +- **Always wrap test runs with `timeout`**: e.g. `timeout 15 <build>/gtest_launchdarkly-cpp-internal`. Deadlocks don't fail GTest — they hang forever. |
| 14 | +- **After adding a brand-new test file, re-run `cmake -S . -B <build-dir>`** before `cmake --build`. The test CMakeLists use `file(GLOB)`, which only re-globs at configure time; without a re-configure, the new file is silently skipped. |
| 15 | +- Test target names: `gtest_launchdarkly-cpp-{common,internal,client,server,sse-client}`. Note the `-cpp-server` form — NOT `gtest_launchdarkly-server-sdk`. |
| 16 | +- Test binaries land at the build root (`<build>/gtest_…`), not nested under the source tree's `tests/` directory. |
| 17 | + |
| 18 | +## Client vs server SDKs |
| 19 | + |
| 20 | +`libs/client-sdk` and `libs/server-sdk` are distinct SDKs sharing libraries (`libs/common`, `libs/internal`, `libs/server-sent-events`). Public APIs and data models differ. Always state which SDK(s) a change applies to — the answer is rarely "both," even when the feature name is shared. |
| 21 | + |
| 22 | +## Project layout |
| 23 | + |
| 24 | +- `libs/` — SDK and shared libraries. |
| 25 | +- `architecture/` — design docs for major subsystems; read before non-trivial changes in those areas. |
| 26 | +- `vendor/` — third-party source with project-specific patches; don't edit casually. |
| 27 | +- `contract-tests/`, `examples/`, `scripts/`, `cmake/`, `cmake-tests/` — as named. |
| 28 | + |
| 29 | +## Coding conventions |
| 30 | + |
| 31 | +Basics (Google C++ style, naming, `#pragma once`, `.cpp/.hpp` for C++, `.c/.h` for C) are in `CONTRIBUTING.md`. Repo-specific rules below. |
| 32 | + |
| 33 | +### Includes |
| 34 | + |
| 35 | +- `""` for headers in this library's own source tree (sibling headers, private internal headers). |
| 36 | +- `<>` for the library's installed public API (`<launchdarkly/...>`), third-party (Boost, etc.), and standard library. |
| 37 | +- Group order: std/system, then third-party, then `<launchdarkly/...>`, then `"local"`. Blank line between groups. (Matches existing code.) |
| 38 | + |
| 39 | +### Public APIs |
| 40 | + |
| 41 | +- Anything under `libs/*/include/launchdarkly/...` is public. Avoid breaking changes. If a public signature must change, call it out explicitly in the PR description. |
| 42 | +- Shared-library builds export only the C API. Don't change symbol visibility without understanding why. |
| 43 | + |
| 44 | +### Naming |
| 45 | + |
| 46 | +- Public methods returning `bool` are named `IsX()` / `HasX()`, not `X()`. |
| 47 | + |
| 48 | +### Parameters |
| 49 | + |
| 50 | +- For mutable (out / in-out) parameters, use `T*`, not `T&`. The pointer makes mutation visible at the call site (`&foo` vs `foo`). Const references (`const T&`) for read-only are fine. |
| 51 | +- For literal arguments (`nullptr`, `true`, `false`, integer/duration literals), prefix with `/* name= */` when the parameter's role isn't obvious. Skip when the function name + literal type already convey it (`WaitForResult(1s)`, `unique_ptr(nullptr)`, `vec.reserve(100)`). |
| 52 | + |
| 53 | +### Comments & doc comments |
| 54 | + |
| 55 | +- Default to one-line comments. If you're writing a paragraph, either the name needs to do more work or no comment is needed. |
| 56 | +- Non-trivial classes need a doc comment that explicitly states the **thread-safety contract** (thread-safe / not thread-safe / partial, with specifics on which methods are safe from which contexts). |
| 57 | +- Public methods on non-trivial classes need a doc comment explaining how callers should use them: preconditions, side effects, threading constraints, return semantics. Document **what callers need**, not implementation details (don't expose `shared_ptr`, backends, internal resource management). |
| 58 | +- No Java references. American spellings. |
| 59 | +- No session/roadmap labels in comments ("Step 7", "phase 2"). Use `TODO:` plus a description. |
| 60 | + |
| 61 | +### Memory & ownership |
| 62 | + |
| 63 | +- `shared_ptr` is **not** the default. Each use needs a concrete justification (genuine shared ownership, async callbacks outliving the owner, type-erased callable storage). Default to value, `unique_ptr`, or non-owning `T*` with a documented lifetime contract. |
| 64 | +- Don't take a constructor parameter as `T&` and store it as a member reference. Use `shared_ptr` (with justification), `T*` plus a constructor doc comment on lifetime, or take ownership. |
| 65 | + |
| 66 | +### Thread safety |
| 67 | + |
| 68 | +- `boost::asio::io_context` does **not** serialize callbacks by itself — it can be run from multiple threads. When a thread-safety comment relies on serialization, name the actual mechanism: strand, mutex, single-thread invariant, or sequential `.Then()` chaining. |
| 69 | +- Inside thread-safe classes, group member variables by what protects them (`const`, `protected by mutex_`, etc.) and annotate each group. |
| 70 | +- Default lock scope is the **whole function** with one `std::lock_guard` at the top. Only narrow with a stated reason (re-entrant callback, blocking I/O, lock ordering) — write the reason in a comment. |
| 71 | +- A pointer/reference derived from mutex-protected state inherits the protection. Don't copy a raw pointer out under the lock and use it after releasing. |
| 72 | + |
| 73 | +### Futures |
| 74 | + |
| 75 | +- When chaining with `Future::Then`, use the flattening overload. Don't capture a `Promise` in the continuation to resolve manually. |
| 76 | + |
| 77 | +### Source organization |
| 78 | + |
| 79 | +- Method definitions in `.cpp` follow the order of declarations in the `.hpp`. |
| 80 | + |
| 81 | +## Tests |
| 82 | + |
| 83 | +- GoogleTest. Match the style of the adjacent test file you're modifying. |
| 84 | +- Default to inline construction of test data. Helpers are acceptable when they *substantially* reduce boilerplate repeated across many tests — not just for "I might call this twice." |
| 85 | +- Don't sleep then assert a negative ("future didn't fire", "callback not called"). If nothing else can resolve the future, check state synchronously. |
| 86 | +- Skip `// Act` / `// Assert` labels — write a short descriptive comment of what the block does or expects. |
| 87 | +- Don't pad timer durations "for CI safety" in drain-then-assert tests; 5ms is fine when there's no race to lose. |
| 88 | + |
| 89 | +## Docs |
| 90 | + |
| 91 | +Doxygen, configured per-library via a `Doxyfile`. Build with `scripts/build-docs.sh <library-dir>`. Public APIs get Doxygen comments describing behavior callers depend on — not implementation details. |
| 92 | + |
| 93 | +## Formatting |
| 94 | + |
| 95 | +`clang-format -i <file>` (config is in the repo). |
| 96 | + |
| 97 | +## Process |
| 98 | + |
| 99 | +- **Commit messages:** single-line conventional commits (`feat:`, `fix:`, `refactor:`, `docs:`, `chore:`, `test:`). No body unless asked. |
| 100 | +- **PR descriptions:** terse. One-sentence summary, a few bullets for what's in, design decisions only for non-obvious choices. Add a "not in scope" line only when a reviewer would reasonably expect that thing to be in this PR — don't manufacture exclusions. |
0 commit comments