-
Notifications
You must be signed in to change notification settings - Fork 10
docs: Add submodule initialization instructions to README #172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| name: "build" | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - "README.md" | ||
| - "docs/**/*" | ||
| - "*.md" | ||
| push: | ||
| paths: | ||
| - "README.md" | ||
| - "docs/**/*" | ||
| - "*.md" | ||
|
|
||
| concurrency: | ||
| group: "${{github.workflow}}-${{github.ref}}" | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| build: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: "Documentation-only changes" | ||
| run: echo "Only documentation changed, skipping build" | ||
|
Comment on lines
+20
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| cmake_minimum_required(VERSION 3.22.1) | ||
|
|
||
| # Ensure submodules are initialized before building | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 # 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:
🤖 Prompt for AI Agents |
||
| project( | ||
| log_surgeon | ||
| VERSION 0.0.1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
Also applies to: 15-17
🤖 Prompt for AI Agents