fix: no usage of etc/script/ci/build.sh #173
Conversation
WalkthroughThis pull request introduces changes to build configuration and scripts across multiple files. The primary modifications involve adding compile-time definitions for Git commit ID and build date, updating build scripts to capture and pass build metadata, and removing a redundant CI build script. The changes aim to enhance build information tracking by embedding version and build context directly into the compilation process. Changes
Sequence DiagramsequenceDiagram
participant Build Script
participant Git
participant CMake
participant Compiler
Build Script->>Git: Retrieve commit hash
Build Script->>Git: Get current build time
Build Script->>CMake: Pass build metadata
CMake->>Compiler: Embed metadata as compile-time definitions
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
etc/script/build.sh (2)
17-17: Remove unused variable CONF.The variable
CONFis declared but never used in the script.-CONF="${PROJECT_HOME}/etc/conf/kiwi.conf"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: CONF appears unused. Verify use (or export if used externally).
(SC2034)
19-29: Add error handling for git command failures.While the code handles empty commit IDs, it should also handle git command failures.
-BUILD_TIME=$(git log -1 --format=%ai) +BUILD_TIME=$(git log -1 --format=%ai 2>/dev/null) BUILD_TIME=${BUILD_TIME:0:10} -COMMIT_ID=$(git rev-parse HEAD) +COMMIT_ID=$(git rev-parse HEAD 2>/dev/null) SHORT_COMMIT_ID=${COMMIT_ID:0:8} if [ -z "$SHORT_COMMIT_ID" ]; then - echo "no git commit id" + echo "Warning: Could not determine git commit id" SHORT_COMMIT_ID="kiwi" fiCMakeLists.txt (1)
277-282: Consider using a version file or git tags for version management.While embedding build metadata is good, hardcoding the version number (
4.0.0) makes version management more difficult. Consider:
- Using a separate version file
- Using git tags for version information
- Implementing semantic versioning
Example implementation using a version file:
+# Read version from version file +FILE(STRINGS "VERSION" KIWI_VERSION) target_compile_definitions(kiwi PRIVATE - KIWI_VERSION="4.0.0" + KIWI_VERSION="${KIWI_VERSION}" KIWI_BUILD_TYPE="${BUILD_TYPE}" KIWI_GIT_COMMIT_ID="${GIT_COMMIT_ID}" KIWI_BUILD_DATE="${BUILD_TIME}" )Create a
VERSIONfile in the root directory:#!/bin/bash # Verify if VERSION file exists if [ ! -f "VERSION" ]; then echo "Error: VERSION file not found" exit 1 fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakeLists.txt(1 hunks)etc/script/build.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
etc/script/build.sh
[warning] 10-10: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 17-17: CONF appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (4)
etc/script/build.sh (4)
9-9: LGTM! Good shell scripting practice.Properly quoting the BUILD_TYPE value helps prevent word splitting issues.
32-39: LGTM! Robust CPU core detection.The implementation properly handles both Linux and macOS systems with appropriate fallback to single core.
41-46: LGTM! Comprehensive build information display.Clear output of all relevant build parameters improves debugging and traceability.
58-63: LGTM! Well-structured CMake configuration.The build metadata is properly passed to CMake with appropriate quoting.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
etc/script/build.sh (2)
11-11: Remove or utilize unused variables.The static analysis tool identified two unused variables:
VERBOSE(line 11)CONF(line 18)Consider removing these variables if they are no longer needed, or document their purpose if they are intended for future use.
-VERBOSE=0 -CONF="${PROJECT_HOME}/etc/conf/kiwi.conf"Also applies to: 18-18
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
59-65: Quote CMake variables to handle spaces.The CMake variables passed to the build command should be quoted to handle potential spaces in paths or values.
cmake -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \ - -DK_VERSION=$K_VERSION \ - -DBUILD_TIME=$BUILD_TIME \ - -DGIT_COMMIT_ID=$SHORT_COMMIT_ID \ - -DKIWI_BUILD_DATE="$BUILD_TIME" \ - -DKIWI_GIT_COMMIT_ID="$SHORT_COMMIT_ID" \ - ${CMAKE_FLAGS} -S . -B ${PREFIX} + -DK_VERSION="${K_VERSION}" \ + -DBUILD_TIME="${BUILD_TIME}" \ + -DGIT_COMMIT_ID="${SHORT_COMMIT_ID}" \ + -DKIWI_BUILD_DATE="${BUILD_TIME}" \ + -DKIWI_GIT_COMMIT_ID="${SHORT_COMMIT_ID}" \ + "${CMAKE_FLAGS}" -S . -B "${PREFIX}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CMakeLists.txt(1 hunks)etc/script/build.sh(2 hunks)etc/script/ci/build.sh(0 hunks)src/kiwi.h(1 hunks)
💤 Files with no reviewable changes (1)
- etc/script/ci/build.sh
✅ Files skipped from review due to trivial changes (1)
- src/kiwi.h
🧰 Additional context used
🪛 Shellcheck (0.10.0)
etc/script/build.sh
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: CONF appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
CMakeLists.txt (1)
Line range hint
1-1: Verify removal of etc/script/ci/build.sh.As per the PR objectives and past review comments, etc/script/ci/build.sh is being removed. Let's verify there are no remaining references to this script.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
etc/script/build.sh (2)
9-9: Consider using semantic versioning from git tags.Instead of hardcoding the version, consider deriving it from git tags to ensure consistent version management across the project.
-K_VERSION="1.0.0" +K_VERSION=$(git describe --tags 2>/dev/null || echo "1.0.0")
36-43: Enhance CPU core detection for better cross-platform support.The current CPU core detection could be improved to handle more platforms and edge cases.
- if [ ! -f "/proc/cpuinfo" ]; then - CPU_CORE=$(sysctl -n hw.ncpu) - else - CPU_CORE=$(cat /proc/cpuinfo | grep "processor" | wc -l) - fi - if [ ${CPU_CORE} -eq 0 ]; then - CPU_CORE=1 - fi + # Try different methods to detect CPU cores + CPU_CORE=1 + if command -v nproc >/dev/null 2>&1; then + CPU_CORE=$(nproc) + elif [ -f "/proc/cpuinfo" ]; then + CPU_CORE=$(grep -c processor /proc/cpuinfo) + elif command -v sysctl >/dev/null 2>&1; then + CPU_CORE=$(sysctl -n hw.ncpu) + fi + # Ensure at least one core is used + CPU_CORE=$(( CPU_CORE > 0 ? CPU_CORE : 1 ))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakeLists.txt(1 hunks)etc/script/build.sh(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CMakeLists.txt
🧰 Additional context used
🪛 Shellcheck (0.10.0)
etc/script/build.sh
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: CONF appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (4)
etc/script/build.sh (4)
20-33: Well implemented error handling for git commands!The error handling for git commands has been properly implemented, making the script more resilient in non-git environments.
45-50: LGTM! Improved build transparency.The added echo statements provide better visibility into the build configuration.
62-68: LGTM! Proper passing of build metadata.The CMake configuration properly passes all build metadata variables.
158-158: LGTM! Well-structured script with proper error handling.The script demonstrates good practices with clear documentation, proper error handling, and well-structured command-line argument parsing.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CMakeLists.txt (1)
277-282: Consider using existing Git commit hash variable.The CMakeLists.txt already defines
GIT_COMMIT_HASH(line 31-35), but introduces a newGIT_COMMIT_IDvariable. Consider reusing the existing variable to maintain consistency.TARGET_COMPILE_DEFINITIONS(kiwi PRIVATE KIWI_VERSION="${K_VERSION}" KIWI_BUILD_TYPE="${BUILD_TYPE}" - KIWI_GIT_COMMIT_ID="${GIT_COMMIT_ID}" + KIWI_GIT_COMMIT_ID="${GIT_COMMIT_HASH}" KIWI_BUILD_DATE="${BUILD_TIME}" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/import_braft.yml(1 hunks).github/workflows/kiwidb.yml(1 hunks)CMakeLists.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: check_format
🔇 Additional comments (2)
.github/workflows/kiwidb.yml (1)
19-23: Verify ccache integration with direct CMake usage.The workflow uses ccache for build caching, but the direct CMake commands might bypass the ccache configuration set in the environment.
Run this script to check ccache integration:
CMakeLists.txt (1)
277-282: Add checks for required CMake variables.The target compile definitions use variables that must be defined externally. As suggested in the previous review, add existence checks for these variables.
Run this script to verify variable definitions:
#!/bin/bash # Description: Check if required variables are defined in build.sh # Check for version, build time and commit ID definitions rg "K_VERSION.*=.*1\.0\.0" ./etc/script/build.sh rg "BUILD_TIME.*=.*date" ./etc/script/build.sh rg "GIT_COMMIT_ID.*=.*git" ./etc/script/build.sh
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
etc/script/build.sh (1)
11-11: 🛠️ Refactor suggestionRemove unused variables.
The variables
VERBOSEandCONFare defined but never used in the script.-VERBOSE=0 CMAKE_FLAGS="" MAKE_FLAGS="" PREFIX="cmake-build" PWD=$(pwd) PROJECT_HOME="${PWD}/" -CONF="${PROJECT_HOME}/etc/conf/kiwi.conf"Also applies to: 18-18
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
🧹 Nitpick comments (3)
etc/script/get_gitinfo_func.sh (1)
16-19: Improve error messaging for unknown commit ID.The error message "no git commit id" doesn't provide enough context about why the commit ID is unknown (e.g., not a git repository, git command failed, etc.).
if [ "$SHORT_COMMIT_ID" = "unknown" ]; then - echo "no git commit id" + echo "Warning: Could not determine git commit ID (not a git repository or git command failed)" SHORT_COMMIT_ID="kiwi" fietc/script/build.sh (2)
54-60: Remove duplicate cmake definitions.The script defines both
-DBUILD_TIMEand-DKIWI_BUILD_DATEwith the same value, and similarly for-DGIT_COMMIT_IDand-DKIWI_GIT_COMMIT_ID. This appears redundant.cmake -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \ -DK_VERSION=$K_VERSION \ - -DBUILD_TIME=$BUILD_TIME \ - -DGIT_COMMIT_ID=$SHORT_COMMIT_ID \ -DKIWI_BUILD_DATE="$BUILD_TIME" \ -DKIWI_GIT_COMMIT_ID="$SHORT_COMMIT_ID" \ ${CMAKE_FLAGS} -S . -B ${PREFIX}
28-35: Improve CPU core detection logic.The CPU core detection could be improved by:
- Using
nproccommand which is more portable- Adding a maximum core limit to prevent overloading
- if [ ! -f "/proc/cpuinfo" ]; then - CPU_CORE=$(sysctl -n hw.ncpu) - else - CPU_CORE=$(cat /proc/cpuinfo | grep "processor" | wc -l) - fi - if [ ${CPU_CORE} -eq 0 ]; then - CPU_CORE=1 - fi + # Get available CPU cores (fallback to nproc if available, else 1) + CPU_CORE=$(nproc 2>/dev/null || echo 1) + # Limit maximum cores to prevent overload + MAX_CORES=8 + if [ ${CPU_CORE} -gt ${MAX_CORES} ]; then + CPU_CORE=${MAX_CORES} + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
etc/script/build.sh(2 hunks)etc/script/ci/build.sh(2 hunks)etc/script/get_gitinfo_func.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
etc/script/build.sh
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: CONF appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
etc/script/ci/build.sh (1)
Line range hint
1-21: Remove this file as it's no longer needed.As per the PR objective and previous review comments, this file should be removed since its functionality has been moved to
etc/script/get_gitinfo_func.sh.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/import_braft.yml (1)
Line range hint
58-58: Fix typo in build command argument.The argument
--verbosappears to be a typo. It should be--verboseto match the available options in build.sh.- bash ./etc/script/build.sh --verbos + bash ./etc/script/build.sh --verbose
🧹 Nitpick comments (2)
etc/script/build.sh (2)
20-41: Improve error handling in get_git_info function.While the function handles command failures, it could be improved by:
- Adding logging for better debugging
- Validating the git repository existence upfront
function get_git_info() { + if ! git rev-parse --git-dir > /dev/null 2>&1; then + echo "Warning: Not a git repository" + export BUILD_TIME="unknown" + export SHORT_COMMIT_ID="kiwi" + return + fi + time=$(git log -1 --format=%ai 2>/dev/null || echo "unknown") if [ "$time" != "unknown" ]; then time=${time:0:10} + echo "Build time set to: $time" fi export BUILD_TIME=$time id=$(git rev-parse HEAD 2>/dev/null || echo "unknown") id=$([ "$id" != "unknown" ] && echo ${id:0:8} || echo "unknown") if [ "$id" = "unknown" ]; then echo "no git commit id" id="kiwi" + else + echo "Commit ID set to: $id" fi export SHORT_COMMIT_ID=$id - # echo "$BUILD_TIME $SHORT_COMMIT_ID" }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 21-21: SCRIPT_DIR appears unused. Verify use (or export if used externally).
(SC2034)
171-171: Consider making git info optional.The script always calls
get_git_infobeforebuild, making it redundant to have a separate--gitinfooption. Consider:
- Making git info collection optional via a flag
- Moving the git info collection into the build function
-get_git_info +# Only collect git info if not already set +if [ "$BUILD_TIME" = "unknown" ] || [ "$SHORT_COMMIT_ID" = "unknown" ]; then + get_git_info +fi build
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/import_braft.yml(1 hunks)etc/script/build.sh(4 hunks)etc/script/ci/build.sh(0 hunks)
💤 Files with no reviewable changes (1)
- etc/script/ci/build.sh
🧰 Additional context used
🪛 Shellcheck (0.10.0)
etc/script/build.sh
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: CONF appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 21-21: SCRIPT_DIR appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (3)
.github/workflows/import_braft.yml (1)
17-17: LGTM! Build command updated to use new gitinfo functionality.The change aligns with the new
get_git_infofunction in build.sh.etc/script/build.sh (2)
9-14: Remove unused variables.The variables
VERBOSEandCONFare defined but never used in the script.-VERBOSE=0 CMAKE_FLAGS="" MAKE_FLAGS="" PREFIX="cmake-build" PWD=$(pwd) PROJECT_HOME="${PWD}/" -CONF="${PROJECT_HOME}/etc/conf/kiwi.conf"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
70-76: Verify the consistency of build metadata variables.The script defines duplicate variables for build date and commit ID:
BUILD_TIMEvsKIWI_BUILD_DATEGIT_COMMIT_IDvsKIWI_GIT_COMMIT_ID✅ Verification successful
Build metadata variables follow a valid pattern
The duplication is intentional and follows good practices:
- Build system variables (BUILD_TIME, GIT_COMMIT_ID) are mapped to application-specific versions (KIWI_*)
- This provides a clean interface between build system and application code
- Prefixed versions in C++ code prevent naming conflicts and follow macro naming conventions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if these variables are used differently in CMake # Search for both variants in CMake files rg "K(IWI)?_(BUILD_DATE|GIT_COMMIT_ID)" -t cmakeLength of output: 166
Script:
#!/bin/bash # Search for all occurrences of these variables in the entire codebase rg "K(IWI)?_(BUILD_DATE|GIT_COMMIT_ID)" -g '!{etc/script/build.sh,CMakeLists.txt}' # Also search for the non-KIWI prefixed versions rg "BUILD_TIME|GIT_COMMIT_ID" -g '!{etc/script/build.sh,CMakeLists.txt}'Length of output: 1919
AlexStocks
force-pushed
the
fix-merge-build-sh
branch
from
bf25c50 to
0db5c0d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
etc/script/build.sh (3)
40-40: Remove commented debug code.The commented debug line can be safely removed as it's not providing value.
- # echo "$BUILD_TIME $SHORT_COMMIT_ID"
44-51: Add fallback for CPU core detection.The CPU core detection could fail silently if both methods fail. Consider adding a warning message.
if [ ! -f "/proc/cpuinfo" ]; then CPU_CORE=$(sysctl -n hw.ncpu) else CPU_CORE=$(cat /proc/cpuinfo | grep "processor" | wc -l) fi if [ ${CPU_CORE} -eq 0 ]; then + echo "Warning: Could not detect CPU cores, defaulting to 1" CPU_CORE=1 fi
57-58: Quote variables in echo statements to preserve formatting.Variables that might contain spaces should be quoted in echo statements.
- echo "CMAKE_FLAGS: $CMAKE_FLAGS" - echo "MAKE_FLAGS: $MAKE_FLAGS" + echo "CMAKE_FLAGS: '$CMAKE_FLAGS'" + echo "MAKE_FLAGS: '$MAKE_FLAGS'"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/import_braft.yml(1 hunks)etc/script/build.sh(4 hunks)etc/script/ci/build.sh(0 hunks)
💤 Files with no reviewable changes (1)
- etc/script/ci/build.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/import_braft.yml
🧰 Additional context used
🪛 Shellcheck (0.10.0)
etc/script/build.sh
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: CONF appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 21-21: SCRIPT_DIR appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (4)
etc/script/build.sh (4)
11-18: Remove unused variables.The following variables are defined but never used in the script:
VERBOSE(line 11)CONF(line 18)SCRIPT_DIR(line 21)Consider removing them or documenting their intended usage.
Also applies to: 21-21
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: VERBOSE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: CONF appears unused. Verify use (or export if used externally).
(SC2034)
26-41: LGTM! Robust error handling implementation.The
get_git_infofunction properly handles git command failures and provides meaningful defaults. Good job implementing:
- Fallback to "unknown" when git commands fail
- Proper error message when commit ID is unavailable
- Safe variable exports
Line range hint
104-113: LGTM! Clean implementation of the gitinfo option.The new
--gitinfooption is well integrated:
- Clear help text documentation
- Proper argument handling
- Logical placement in the command processing
Also applies to: 127-130
171-172: LGTM! Proper initialization sequence.The script correctly initializes git information before starting the build process.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/kiwidb.yml (1)
Line range hint
134-138: Add error handling to macOS Go E2E tests.The Ubuntu job properly handles Go test failures with
|| exit 1, while the macOS job might pass even if tests fail. This inconsistency could mask test failures in the macOS builds.Apply this diff to align error handling with the Ubuntu job:
run: | set +e cd ../tests go mod tidy - go test -timeout 15m --ginkgo.v + go test -timeout 15m --ginkgo.v || exit 1 sh print_log.sh
🧹 Nitpick comments (1)
.github/workflows/kiwidb.yml (1)
Line range hint
1-1: Document build script parameters in workflow comments.The workflow uses different build script parameters (
gitinfoand--verbose) but lacks documentation explaining their purpose and when to use each. Consider adding workflow-level comments to document:
- The purpose of each parameter
- When to use
gitinfovs--verbose- Impact on build outputs
Add this documentation at the top of the workflow:
name: kiwi + +# Build script parameters: +# - gitinfo: Captures Git commit information and build time +# - --verbose: Enables detailed build output logging
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/kiwidb.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: check_format
🔇 Additional comments (1)
.github/workflows/kiwidb.yml (1)
20-20: Ensure consistent build script parameters across jobs.The
check_formatjob usesgitinfoparameter whilebuild_on_macosandbuild_on_ubuntujobs use--verbose. This inconsistency might lead to different build behaviors across jobs.Let's verify the build script parameters:
#171
Summary by CodeRabbit
New Features
Chores
Documentation