Conversation
Two design documents for community discussion, not intended to merge: restructure-draft-v2.md defines the target file structure and namespace hierarchy for 6.0.0 -- a single include root, one directory and nested namespace per sketch family, and an internal/ subdirectory for everything outside the supported API. It records 18 ratified decisions and 3 open items. restructure-plan-detail.md holds the sequencing: the PR order from the C++17 flag flip through the move, the namespace rewrite and cleanup, plus a per-area breakdown and the techniques for keeping each PR reviewable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Satisfies the RAT license audit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
||
| ``` | ||
| include/datasketches/… the library — the only thing that is installed | ||
| test/datasketches/… mirrors include/datasketches/ |
There was a problem hiding this comment.
Personal preference: could we use test/<area>/ instead? It would still mirror the sketch-family layout without the extra datasketches/ directory.
There was a problem hiding this comment.
Good question. The extra level is there to keep path ≡ namespace (convention 7) for tests too: each test file opens datasketches::, so test/datasketches/theta/ lines up the same way include/datasketches/theta/ does. It also leaves test/ itself free for things that don't mirror the include tree (CMake, TCK output, fixtures tooling) without them looking like areas. Helper includes are the same either way (<common/test_allocator.hpp>), so the only cost is one directory level. I'd lean toward keeping it, but it's a preference-level call and I'm open if others feel strongly.
| │ ├── quantiles_sorted_view.hpp / _impl exported by kll, quantiles, req | ||
| │ └── kolmogorov_smirnov.hpp / _impl user-called; compares two quantiles_sorted_views | ||
| │ | ||
| ├── internal/ datasketches::internal |
There was a problem hiding this comment.
Personal preference: I’d lean toward detail rather than internal. Both convey the intent, and detail keeps paths and qualified names a little shorter.
There was a problem hiding this comment.
detail has good precedent (Boost, libstdc++), but I'd prefer to stay with internal. The saving is two characters per qualified name, and internal explains itself to someone reading a path or an error message without knowing the Boost convention. It also has solid C++ precedent in Abseil (absl/container/internal/) and Arrow (arrow::compute::internal), and it's the word Java developers already know (jdk.internal). Most of our contributors work in both languages, so a term that means the same thing on both sides helps with cross-language work. Unless reviewers have a strong preference otherwise, I'd like to keep internal.
This branch has not been deployed
2 participants
Two design documents for the 6.0.0 restructure, posted here so they can be commented on line by line. This PR is not intended to merge — it is a place to mark up the text. The decision thread will be on dev@datasketches.apache.org
restructure-draft-v2.md (pretty copy, the markup review copy is below) — the target file structure and namespace hierarchy: a single include root, one directory and nested namespace per sketch family, and an
internal/subdirectory for everything outside the supported API. Every header in the current tree is accounted for: kept, moved, renamed, split, or deleted. Includes the precedent in other libraries, 18 ratified decisions and 3 open items.restructure-plan-detail.md (pretty copy, the markup review copy is below) — the sequencing: the PR order from the C++17 flag flip through the move, the namespace rewrite and the cleanup, plus a per-area breakdown and the techniques for keeping each PR reviewable.
Comments inline on the files are very welcome, particularly on the ratified decisions and the open items.
🤖 Generated with Claude Code