Repository structure modernization
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Aptitud para principiantes
- 35/100
- Tipo de issue
- Refactorización
- Claridad
- Necesita aclaración
- Estado de actividad
- Activo
- Stack tecnológico
- cmake, cpp
Línea de trabajo
Empieza leyendo hll/CMakeLists.txt y los archivos CMakeLists de include y test de cada sketch mencionados en la issue. Sigue qué headers se instalan y cómo se repite el descubrimiento de tests; después compara las disposiciones de include y los directorios detail propuestos con los consumidores actuales. Se considera terminado cuando el proyecto acuerda un destino y el alcance de la migración antes de que comience la implementación.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
We'd like to discuss two friction points in the repo layout, both about the public include surface. Filing as a discussion because any change here would break the public API and warrant a major version bump; better to agree on the destination before anyone writes a patch.
Problem 1: Generic top-level header names collide across libraries
Public headers sit at shallow paths with generic filenames, installed flat under a single directory:
hll/include/hll.hpp
theta/include/theta_sketch.hpp
common/include/serde.hpp
common/include/optional.hpp
# ...installed via `DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/DataSketches"`
Consumers write #include <hll.hpp>. Nothing in the path identifies the library, and names like hll.hpp, serde.hpp, optional.hpp, and tdigest.hpp are plausible choices for any number of unrelated projects. A consumer linking against datasketches alongside another library that also ships an hll.hpp ends up with two headers competing for the same include path, with no clean way to disambiguate.
The fix is to namespace the include path under the library name. Two shapes are worth considering.
Option A: Nested per-sketch directories, drop the filename prefix
include/datasketches/
hll/
sketch.hpp # was hll.hpp
detail/...
theta/
sketch.hpp # was theta_sketch.hpp
union.hpp # was theta_union.hpp
intersection.hpp
a_not_b.hpp
jaccard_similarity.hpp
detail/...
cpc/
sketch.hpp
detail/...
...
Consumer: #include <datasketches/theta/union.hpp>
The directory does the namespacing work; the filename drops the now-redundant theta_ prefix. Matches Boost (<boost/graph/adjacency_list.hpp>).
Option B: Flat namespace, keep descriptive filenames
include/datasketches/
hll_sketch.hpp # was hll.hpp
theta_sketch.hpp
theta_union.hpp
theta_intersection.hpp
theta_a_not_b.hpp
theta_jaccard_similarity.hpp
cpc_sketch.hpp
...
detail/
hll/...
theta/...
Consumer: #include <datasketches/theta_union.hpp>
The filename prefix carries the grouping, and the public surface stays on a single flat level. Matches Abseil (<absl/container/flat_hash_map.h>).
Either way, the install path becomes ${CMAKE_INSTALL_INCLUDEDIR}/datasketches/.
Problem 2: Public and internal headers live in the same directory
hll/include/ mixes the public hll.hpp with roughly 30 implementation headers (e.g. HllSketchImpl.hpp, AuxHashMap.hpp) plus another 15-odd *-internal.hpp files. All of them get installed by hll/CMakeLists.txt. From a consumer standpoint, this seems valid:
#include <DataSketches/HllSketchImpl.hpp>
#include <DataSketches/AuxHashMap-internal.hpp>
There is no clear API boundary between public and internal headers.
The fix is a per-sketch detail/ subdirectory, a convention Boost has used since the early 2000s and that Abseil mirrors with its internal/ directories. Implementation headers move there, and the contract for consumers is straightforward: anything inside detail/ is internal, and reaching in is at your own risk.
Low-priority items
- Header guards. Two styles coexist:
hll/include/HllArray.hppuses_HLLARRAY_HPP_(leading underscore, reserved by the standard);theta/include/theta_sketch.hppusesTHETA_SKETCH_HPP_. Switching all headers to#pragma onceremoves the inconsistency, the reserved-identifier risk, and a class of copy-paste mistakes. - No
.clang-format. Style is enforced by convention only. Committing a single file would replace formatting back-and-forth in code review. - Pick one casing convention. File names and code currently mix CamelCase and snake_case. Low-priority but easy on the eyes.
- Tests scattered per-sketch. Each
<sketch>/test/CMakeLists.txtrepeats the Catch2 wiring and test-discovery setup. Consolidating into a singletest/tree (with per-sketch subdirs) would declare the test framework once, give shared fixtures (random key generators, serde helpers) an obvious home. The current setup works; this is cleanup, not a fix.
- Lenguaje dominante
- C++
- Estrellas
- 274
- Forks
- 89
- Merge medio
- 1 d 16 h
- PR fusionados (30 d)
- 10
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Sin plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de apache/datasketches-cpp
-
Dificultad 4/5 3-5 días Aptitud para principiantes 68/100
apache/datasketches-cpp#533 · 2 comentarios ·
Los mantenedores suelen responder en 1 día
-
Reorganization proposalAbierto
Dificultad 5/5 Más de una semana Aptitud para principiantes 20/100
apache/datasketches-cpp#419 · 6 comentarios ·
Los mantenedores suelen responder en 1 día
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 25/100
apache/datasketches-cpp#416 · 13 comentarios ·
Los mantenedores suelen responder en 1 día
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 25/100
apache/datasketches-cpp#157 · 4 comentarios ·
Los mantenedores suelen responder en 1 día
Todos los issues de apache/datasketches-cpp
Issues similares
-
HasBacktrace Priority-Critical
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
azerothcore/azerothcore-wotlk#27921 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
Los mantenedores suelen responder en 1 día
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 88/100
yhirose/cpp-peglib#344 ·
-
bug-unconfirmed
Dificultad 2/5 1-3 horas Aptitud para principiantes 76/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
shadps4-emu/shadps4-qtlauncher#453 ·
Los mantenedores suelen responder en 2 días