elastic / elastic/ml-cpp

[ML] Add tzdata as test target requirement for Linux and MacOS

Offen
#2,771 2 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
>build v8.17.0
Vorherrschende Sprache
C++
Sterne
157
Forks
67
Ø Merge
17 Std. 52 Min.
Gemergte PRs (30 T.)
20

Beschreibung

In our instructions to build a dev environment, we mention that `tzdata` is required to run some unit tests that do data conversion. However, this instruction is easy to overlook, which leaves developers with failed test without an obvious reason or error message.

I suggest to add `tzdata` as a CMake requirement for building test targets that perform corresponding data time transformations (ml::core?). Unfortunately, `tzdata` is not a library or binary, but a system package. Nonetheless, we could check if the timezone databases exist in the typical system paths and output a meaningful error message if not.

For example:
```CMake
find_file(TZDATA_FILE NAMES "UTC" PATHS "/usr/share/zoneinfo" "/usr/lib/zoneinfo")

if(NOT TZDATA_FILE)
message(FATAL_ERROR "tzdata is not installed or not found. Please install tzdata on your system.")
else()
message(STATUS "tzdata found.")
endif()
```

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginne damit, die CMake-Definition für die Testziele zu finden, die ml::core verwenden, sowie die Anweisungen für die Entwicklungsumgebung, in denen tzdata erwähnt wird. Prüfe, wie diese Ziele unter Linux und macOS konfiguriert sind, und verifiziere anschließend, dass fehlende Zeitzonendatenbanken einen klaren Konfigurationsfehler erzeugen und dass die betroffenen Tests weiterhin konfiguriert werden können, wenn tzdata vorhanden ist.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
cmake, cpp
Bereich
build-system, testing-qa
Issue-Typ
Feature
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.