Skip to content

docs: Add submodule initialization instructions to README#172

Open
jackluo923 wants to merge 3 commits intoy-scope:mainfrom
jackluo923:improve-compilation-docs
Open

docs: Add submodule initialization instructions to README#172
jackluo923 wants to merge 3 commits intoy-scope:mainfrom
jackluo923:improve-compilation-docs

Conversation

@jackluo923
Copy link
Member

@jackluo923 jackluo923 commented Oct 31, 2025

Description

This PR adds clear instructions for initializing Git submodules in the build section of the README. Currently, users who clone the repository will encounter build failures if they don't initialize the yscope-dev-utils submodule before building.

The new instructions guide users to run git submodule update --init --recursive after cloning the repository, ensuring the submodule is properly initialized before proceeding with the build steps.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Verified the submodule initialization command is correct and uses the --recursive flag
  • Confirmed the instructions are placed logically in the README (after requirements section, before build commands)
  • Checked that the markdown formatting renders properly

Summary by CodeRabbit

  • Documentation
    • README updated with a submodule initialization step and concrete install/build workflows, including separate release and debug build instructions.
  • Chores
    • Added a CI workflow that detects documentation-only changes and skips the full build to reduce unnecessary runs.

Include `git submodule update --init --recursive` command in the build
instructions to ensure users properly initialize the yscope-dev-utils
submodule before building the project.
@jackluo923 jackluo923 requested a review from a team as a code owner October 31, 2025 15:47
@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025

Walkthrough

Added README instructions to initialize git submodules and concrete Taskfile commands for install/build (release/debug). Added a new GitHub Actions workflow build-docs-only.yaml that triggers on docs-only changes and runs a single job that prints an informational message and skips the full build. Added a non-functional comment to CMakeLists.txt about submodule initialization.

Changes

Cohort / File(s) Change Summary
Documentation updates
README.md
Added git submodule update --init --recursive after Requirements; replaced placeholder install instructions with task log-surgeon:install-release INSTALL_PREFIX="$HOME/.local"; added task log-surgeon:build-release and task log-surgeon:build-debug and separated build/install examples.
CI workflow
.github/workflows/build-docs-only.yaml
Added new GitHub Actions workflow that triggers on PRs and pushes when documentation files change (README.md, docs/**, *.md), uses concurrency to cancel in-progress runs, and runs a single job which prints a message that only docs changed and the build is skipped.
Build configuration comment
CMakeLists.txt
Inserted a comment: "Ensure submodules are initialized before building" after the minimum required CMake version (no functional changes).

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer (push/PR)
    participant GH as GitHub Actions
    participant Job as build-docs-only job

    Dev->>GH: push / open PR (files changed: README.md or docs/** or *.md)
    GH->>GH: evaluate workflow triggers (docs-only patterns)
    alt matches docs-only
        GH->>Job: start job
        Job->>Job: print "Documentation-only changes — skipping full build"
        Job-->>GH: job completes
    else not docs-only
        Note right of GH: Other workflows may run (not shown)
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check README.md command snippets and placement for accuracy and consistency with project Taskfile.
  • Verify .github/workflows/build-docs-only.yaml trigger patterns and concurrency settings.
  • Confirm CMakeLists.txt comment is non-functional and correctly positioned.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "docs: Add submodule initialization instructions to README" directly aligns with the primary objective of the changeset, which is to add explicit instructions for initializing the Git submodule yscope-dev-utils to the README's build section. The title is specific and clear, using concrete terminology that accurately describes what was changed (submodule initialization instructions) and where (README). While the PR includes supporting changes to other files such as a new GitHub Actions workflow and a comment in CMakeLists.txt, these are secondary to the main documentation update that the title correctly identifies.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Create a lightweight build workflow that reports success for PRs that
only modify documentation files (README.md, docs/**/*). This allows
doc-only PRs to pass the required "build" check without running the
full build and test suite, while code changes still trigger the
complete build process.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0ff14e and 61ab551.

📒 Files selected for processing (1)
  • .github/workflows/build-docs-only.yaml (1 hunks)
🔇 Additional comments (1)
.github/workflows/build-docs-only.yaml (1)

3-13: Path filter patterns are reasonable.

The path filters correctly target documentation changes (README.md, docs/**, and .md files). This ensures the workflow runs only when documentation-related files are modified, aligning with the PR objective to skip expensive builds for docs-only changes.

@@ -0,0 +1,24 @@
name: "build"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Workflow name collision risk with concurrency group.

The workflow is named "build" (line 1), but concurrency group uses ${{github.workflow}} (line 16). If another "build" workflow exists in the repository, both will share the same concurrency group, potentially causing the docs-only workflow to cancel in-progress actual builds or vice versa.

Rename this workflow to something distinct, e.g., "docs-only-check" or "build-docs-only", to ensure independent concurrency handling.

Apply this diff to fix the workflow name:

-name: "build"
+name: "build-docs-only"

Also applies to: 15-17

🤖 Prompt for AI Agents
.github/workflows/build-docs-only.yaml lines 1 and 15-17: the workflow is named
"build" which collides with other workflows when the concurrency group uses `${{
github.workflow }}`; rename the workflow to a unique name like "build-docs-only"
(or "docs-only-check") by changing the name value on line 1 and keep the
existing concurrency group unchanged so it will resolve to the new unique name,
ensuring this workflow no longer shares a concurrency group with other "build"
workflows.

Comment on lines +20 to +24
build:
runs-on: ubuntu-latest
steps:
- name: "Documentation-only changes"
run: echo "Only documentation changed, skipping build"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Minimal informational workflow is acceptable.

The job simply echoes a message indicating that only documentation changed and the build is skipped. This satisfies the requirement of having at least one job in the workflow (GitHub Actions requires this). The informational step serves as a clear signal in CI logs.

Consider whether more comprehensive documentation checks (e.g., link validation, Markdown linting) should be added in the future, but the current approach is acceptable as a foundation.

🤖 Prompt for AI Agents
.github/workflows/build-docs-only.yaml lines 20-24: The workflow currently
contains a single informational job that echoes a message when only
documentation changes are detected; leave this minimal job as-is to satisfy
GitHub Actions requirement, or if desired add future steps for doc-quality
checks (e.g., markdownlint, link checker) by inserting additional steps under
the job to run those tools and fail on errors.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61ab551 and bb3dce5.

📒 Files selected for processing (1)
  • CMakeLists.txt (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T02:00:47.620Z
Learnt from: davidlion
Repo: y-scope/log-surgeon PR: 142
File: taskfiles/examples.yaml:63-76
Timestamp: 2025-07-22T02:00:47.620Z
Learning: CMake's find_package() command automatically honors <PackageName>_ROOT variables (e.g., LOG_SURGEON_ROOT) during its search process. The CMakeLists.txt file does not need to explicitly reference these _ROOT variables - they are built into CMake's package discovery mechanism.

Applied to files:

  • CMakeLists.txt

@@ -1,5 +1,6 @@
cmake_minimum_required(VERSION 3.22.1)

# Ensure submodules are initialized before building
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding CMake verification logic alongside the comment.

While the comment provides helpful guidance, it is non-functional and will not prevent build failures when submodules are missing. Users who encounter initialization errors may not read this comment until after the build fails.

A more robust approach would add CMake logic to detect uninitialized submodules early and emit a clear, actionable error message pointing users to the README or documentation.

Consider adding a check similar to the following before or after the project() call:

# Check if the submodule is initialized
if(NOT EXISTS "${CMAKE_SOURCE_DIR}/build/deps/yscope-dev-utils/CMakeLists.txt")
    message(FATAL_ERROR
        "Git submodule 'yscope-dev-utils' is not initialized. "
        "Please run: git submodule update --init --recursive"
    )
endif()

This ensures that:

  • The error is caught early during CMake configuration
  • The error message provides the exact command users need to run
  • The build fails immediately with clear guidance instead of failing later with cryptic errors
🤖 Prompt for AI Agents
In CMakeLists.txt around line 3, the comment warns about uninitialized git
submodules but provides no enforcement; add a CMake configuration-time check
that verifies the expected submodule path exists and, if missing, emits a clear
fatal error instructing users to run the proper git submodule update --init
--recursive command (or pointing to README) so configuration fails early with an
actionable message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant