Update scaffold templates, metadata, and staging scripts with naming and debug changes #25
Conversation
|
""" WalkthroughThis change removes several legacy staging and consolidation scripts, introduces new scripts for module setup, validation, promotion, and consolidation, and updates the module scaffolding process. It also adds a self-test to the debug utility, deletes its standalone test script, and updates generated documentation and script templates for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant 00_setup_module.sh
participant StagingDir
participant 02_validate_module.sh
participant 04_promote_module.sh
participant SrcDir
participant DocsDir
User->>00_setup_module.sh: setup_module <modulename>
00_setup_module.sh->>StagingDir: Create .conf, .sh, docs_*.md
User->>02_validate_module.sh: validate_module <modulename>
02_validate_module.sh->>StagingDir: Check .conf, .sh, docs_*.md
User->>04_promote_module.sh: promote_module
04_promote_module.sh->>DocsDir: Move docs_*.md
04_promote_module.sh->>SrcDir: Move .sh and .conf based on parent
sequenceDiagram
participant User
participant 06_consolidate_module.sh
participant SrcDir
participant LibDir
User->>06_consolidate_module.sh: consolidate_module
06_consolidate_module.sh->>SrcDir: Read .sh and .conf files
06_consolidate_module.sh->>LibDir: Write consolidated .sh and arrays
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
![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 (1)
tools/staging_00_setup_scaffold.sh (1)
41-42: Alignhelperswith default override semantics.All other metadata keys in this template use the
${key:-default}pattern to allow consumer overrides;helpersis currently a static assignment. You might want to support the same override behavior:-helpers=_about_${MODULE} +helpers=\${helpers:-_about_${MODULE}}
deleted: tests/test_debug.sh modified: tools/consolidate_00_lib_src.sh modified: tools/consolidate_01_lib_metadata.sh modified: tools/staging_00_setup_scaffold.sh modified: tools/staging_01_check_required.sh modified: tools/staging_03_promote_module.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/initialize/debug.sh (3)
38-49: Review here-document delimiter in_about_debug
The change from<<-EOFto<<EOFremoves leading-tab stripping, so any tabs before help lines will be retained. Consider either switching back to<<-EOF(to allow indented here-docs) or removing all tabs and left-aligning the help text.
58-63: Simplify assertion with shell built-in
Instead of spawninggrep, you can use a shell pattern match:if [[ "$help_output" != *"Usage: debug"* ]]; then echo "Help output does not contain expected usage string" exit 1 fiThis avoids an external process and streamlines the self-test.
66-69: Preserve help text formatting and isolate debug output
Usingdebug "$help_output"treats the entire multi-line help text as a singlecmdand runs it through the elapsed-time formatter, which mangles the layout. Also, test and debug messages on stdout can interfere with consumers of the script’s output. Update to:- debug "$help_output" + echo "$help_output" >&2And consider redirecting the other
debug/printfcalls in the self-test to stderr (>&2).tools/consolidate_01_lib_metadata.sh (2)
10-10: Unused IGNORE_FILES variable
IGNORE_FILESis declared but never referenced. Remove it or implement its intended ignore logic to avoid confusion.
12-12: Duplicate directory creation
Themkdir -p "$LIB_ROOT"here duplicates the latermkdir -p "$(dirname "$OUT_FILE")". Consider removing the early call to DRY up the code.tools/staging_03_promote_module.sh (3)
8-9: Remove unused declarations
array_entriesandgroup_countsare declared but never used in this script—drop them to reduce noise.
13-14: Fix comment to match file extension
The comment refers todocs_*.shbut the loop processesdocs_*.md. Update the comment to reflect.md.
20-22: Review skip condition for docs files
Since thefor sh_fileloop only matches*.sh, the*docs_*.shcheck is redundant. If you intended to exclude docs.md, adjust the pattern or remove this branch.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/initialize/debug.sh(1 hunks)tests/test_debug.sh(0 hunks)tools/consolidate_00_lib_src.sh(1 hunks)tools/consolidate_01_lib_metadata.sh(1 hunks)tools/staging_00_setup_scaffold.sh(3 hunks)tools/staging_01_check_required.sh(4 hunks)tools/staging_03_promote_module.sh(1 hunks)
💤 Files with no reviewable changes (1)
- tests/test_debug.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/staging_00_setup_scaffold.sh
🧰 Additional context used
📓 Path-based instructions (1)
`src/**/*.sh`: Shell script review: - Proper variable quoting - Portable shell features - Security considerations - Error handling patterns
src/**/*.sh: Shell script review:
- Proper variable quoting
- Portable shell features
- Security considerations
- Error handling patterns
src/initialize/debug.sh
🪛 Shellcheck (0.10.0)
tools/staging_01_check_required.sh
[warning] 38-38: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
tools/staging_03_promote_module.sh
[warning] 8-8: array_entries appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 9-9: group_counts appears unused. Verify use (or export if used externally).
(SC2034)
tools/consolidate_01_lib_metadata.sh
[warning] 10-10: IGNORE_FILES appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (8)
tools/consolidate_01_lib_metadata.sh (1)
9-9: Approve addition of LIB_ROOT
CreatingLIB_ROOTcentralizes the library output path for metadata arrays.tools/consolidate_00_lib_src.sh (1)
2-2: Enable strict mode (-euo pipefail)
Good enhancement for robust error handling—make sure downstream code handles unset variables and pipeline failures under this stricter mode.tools/staging_01_check_required.sh (4)
16-23: Update required files to include.confand drop tests
Aligns with scaffold changes: only${module}.shand${module}.confare now mandatory.
26-31: Align optional docs filename
Switched fromdoc_${module}.mdtodocs_${module}.md, matching new conventions.
36-45: Rename main function tocheck_requiredand refine output
Function name and output formatting now mirror other staging scripts.
56-58: Invoke the updatedcheck_requiredfunction
Ensures standalone execution with the new function name.tools/staging_03_promote_module.sh (2)
5-7: Create documentation output directory
InitializingDOC_ROOTand ensuring it exists is correct.
31-35: Approve moving logic for scripts and config
The block correctly moves both the.shand its.confintosrc/$parentwith clear messaging.
modified: tools/staging_03_promote_module.sh
…0_setup_module.sh validate_modulenew file: tools/01_validate_module.sh validate_modulerenamed: tools/staging_03_promote_module.sh -> tools/03_promote_module.sh validate_modulenew file: tools/10_consolidate_module.sh validate_moduledeleted: tools/consolidate_00_lib_src.sh validate_moduledeleted: tools/consolidate_01_lib_metadata.sh validate_moduledeleted: tools/staging_01_check_required.sh
coderabbitai
bot
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
tools/00_setup_module.sh (3)
24-24: Remove trailing whitespace.Line 24 has trailing spaces, which fails the style check.
63-90: Make generated module script executable.Currently the scaffolded
${MODULE}.shis not marked executable. Add achmod +xafter creation so the new module can be run immediately.Example diff:
cat > "${STAGING_DIR}/${MODULE}.sh" <<EOF #!/bin/bash set -euo pipefail ... EOF + chmod +x "${STAGING_DIR}/${MODULE}.sh"
95-95: Remove trailing whitespace.Line 95 ends with an extra space, causing a style error.
tools/03_promote_module.sh (3)
6-6: Remove trailing whitespace.Line 6 contains trailing spaces and incorrect line endings; please strip whitespace and ensure a final newline.
10-11: Drop unused array declarations.
array_entriesandgroup_countsare declared but never used in this script. Removing them will eliminate dead code and ShellCheck warnings.- declare -A array_entries - declare -A group_counts # For unique id per group
14-14: Correct comment typo.The comment reads “Move docs_*.sh scripts” but the script handles
docs_*.mdfiles. Update for clarity.tools/01_validate_module.sh (2)
1-1: Fix line endings and add final newline.The script starts with wrong line endings and no trailing newline. Convert to Unix LF and ensure a final newline to satisfy the style check.
54-55: Clarify or remove outdated comment.The comment “# At top of your script” above
REQUIRED_CONF_FIELDSis misleading in this context. Consider updating or removing it.tools/10_consolidate_module.sh (2)
1-1: Fix line endings and add final newline.The shebang line has incorrect line endings and the file lacks a trailing newline. Convert to Unix LF and append a newline.
85-86: Scopesection_kvto its function.
section_kvis declared globally but only used inside_process_confs(). Mark itlocalto avoid potential conflicts and satisfy stricter shellcheck settings._process_confs() { - declare -A section_kv=() + local -A section_kv=() ... }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tools/00_setup_module.sh(1 hunks)tools/01_validate_module.sh(1 hunks)tools/03_promote_module.sh(1 hunks)tools/10_consolidate_module.sh(1 hunks)tools/consolidate_00_lib_src.sh(0 hunks)tools/consolidate_01_lib_metadata.sh(0 hunks)tools/staging_01_check_required.sh(0 hunks)tools/staging_03_promote_module.sh(0 hunks)
💤 Files with no reviewable changes (4)
- tools/staging_03_promote_module.sh
- tools/consolidate_00_lib_src.sh
- tools/staging_01_check_required.sh
- tools/consolidate_01_lib_metadata.sh
🧰 Additional context used
🪛 GitHub Actions: 01 Coding style check
tools/00_setup_module.sh
[error] 24-24: Wrong line endings or no final newline; Trailing whitespace at line 24.
[error] 95-95: Trailing whitespace at line 95.
tools/03_promote_module.sh
[error] 6-6: Wrong line endings or no final newline; Trailing whitespace at line 6.
tools/01_validate_module.sh
[error] 1-1: Wrong line endings or no final newline.
tools/10_consolidate_module.sh
[error] 1-1: Wrong line endings or no final newline.
🪛 Shellcheck (0.10.0)
tools/03_promote_module.sh
[warning] 10-10: array_entries appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 11-11: group_counts appears unused. Verify use (or export if used externally).
(SC2034)
tools/10_consolidate_module.sh
[warning] 95-95: section_kv appears unused. Verify use (or export if used externally).
(SC2034)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tools/03_promote_module.sh (2)
10-11: Remove unused variable declarations
array_entriesandgroup_countsare declared but never used in this script. Removing them reduces confusion and silences ShellCheck warnings.- declare -A array_entries - declare -A group_counts # For unique id per group
21-27: Extract the staging directory into a variable
Hard-coding./stagingreduces flexibility. Consider definingSTAGING_DIR="${STAGING_DIR:-./staging}"at the top and using it in both loops.tools/10_consolidate_module.sh (1)
1-5: Add a Bash version check fordeclare -n
The script relies on name references (declare -n), which require Bash ≥4.3. Add an early version check to fail on older Bash versions:if (( BASH_VERSINFO[0] < 4 || ( BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 3 ) )); then echo "Error: Bash 4.3+ is required to run this script." >&2 exit 1 fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tools/00_setup_module.sh(1 hunks)tools/01_validate_module.sh(1 hunks)tools/03_promote_module.sh(1 hunks)tools/10_consolidate_module.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/00_setup_module.sh
- tools/01_validate_module.sh
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/10_consolidate_module.sh
[warning] 95-95: section_kv appears unused. Verify use (or export if used externally).
(SC2034)
tools/03_promote_module.sh
[warning] 10-10: array_entries appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 11-11: group_counts appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
tools/10_consolidate_module.sh (1)
95-95: Skip false positive SC2034 forsection_kv
section_kvis passed by name toemit_sectionand consumed there; the ShellCheck SC2034 warning can be safely ignored.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tools/03_promote_module.sh (5)
7-7: Introduce STAGE_ROOT and SRC_ROOT variables for flexibility
Instead of hard-coding./stagingand./src, define defaults like:STAGE_ROOT="${STAGE_ROOT:-./staging}" SRC_ROOT="${SRC_ROOT:-./src}"and use them throughout to allow overrides.
14-14: Fix comment to match code behavior
The comment mentionsdocs_*.shbut the glob isdocs_*.md. Update it to:-# Move docs_*.sh scripts from staging to docs/ +# Move docs_*.md files from staging to docs/
21-23: Remove redundant skip fordocs_*.sh
The condition[[ "$sh_file" == *docs_*.sh ]]will never match since the glob is limited to*.shand docs files use.md. You can safely remove this check.
28-29: Robustly parseparent=in.conf
Consider using a pipeline that handles whitespace and inline comments, e.g.:parent=$(grep -m1 '^parent=' "$conf_file" | cut -d'=' -f2- | tr -d ' ')
30-34: Use$SRC_ROOTvariable for destination
To respect overrides, replace the hard-coded./src/$parentwith"$SRC_ROOT/$parent".
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tools/00_setup_module.sh(1 hunks)tools/03_promote_module.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/00_setup_module.sh
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/03_promote_module.sh
[warning] 10-10: array_entries appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 11-11: group_counts appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (3)
tools/03_promote_module.sh (3)
1-2: Use of strict mode is appropriate
The shebang andset -euo pipefailprovide robust error handling.
15-19: Docs loop correctly moves Markdown files
The loop safely checks existence and movesdocs_*.mdto$DOC_ROOT. Good use of-fand quoting.
43-45: Correct use of execution guard
The check againstBASH_SOURCE[0]ensurespromote_moduleonly runs when the script is executed directly. This pattern is correctly implemented.
…rkflows/05_github_tests-validate.yml.hold
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/10_consolidate_module.sh (1)
104-112: Consider replacingecho -ewithprintffor portability
While Bash’secho -eworks here,printfis more predictable across environments.- echo -e "######## Auto-generated. Do not edit. ########\n" + printf "######## Auto-generated. Do not edit. ########\n\n"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tools/10_consolidate_module.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/10_consolidate_module.sh
[warning] 95-95: section_kv appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (9)
tools/10_consolidate_module.sh (9)
1-11: Strict mode and globals setup
Utilizingset -euo pipefailwith clearly definedSRC_ROOT,OUT_FILE, and ignore lists establishes a robust foundation for the script.
12-17: Helper and counter arrays initialization
Declaringarray_entries,group_counts, and therequired_helper_keysupfront enhances readability and enforceability of metadata rules.
18-45: Consolidate source files logic is clear
Function_consolidate_srcproperly iterates subdirectories, skips ignored files, strips shebangs, and aggregates module scripts.
47-78: Metadata emission logic is sound
emit_sectioncorrectly validates required helper keys, computes unique IDs, and appends key/value pairs into the appropriate associative arrays.
80-100: Configuration parsing loop is robust
Function_process_confsreads.conffiles section-by-section, handles comments, and ensures the last section is emitted.
85-95: False positive: 'section_kv' flagged as unused
Although ShellCheck raises SC2034,section_kvis passed by nameref toemit_sectionand used indirectly. This warning can be safely ignored.
102-114: Array writing implementation is reliable
_write_arrayswrites sorted associative arrays with clear start/finish markers, promoting maintainability of the generated file.
116-135: Command dispatcher covers expected workflows
consolidate_modulecleanly handlesconsolidate,generate/process, andallmodes with a helpful usage message.
138-141: Self-execution guard is correct
The check on${BASH_SOURCE[0]}ensures the script runs its default behavior only when invoked directly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tools/10_consolidate_module.sh (5)
18-48: Refine directory traversal & output formatting
- Add a guard to skip non-directories in the loop to avoid a literal glob when no subdirs exist.
- Prefer
printfoverecho -efor portability.- Minor grammar tweak in the success message.
Example diff:
@@ -20,6 +20,7 @@ _consolidate_src() { for DIR in "$SRC_ROOT"/*/; do + [[ -d "$DIR" ]] || continue local NS OUT f fname skip relpath NS=$(basename "$DIR") OUT="$LIB_ROOT/$NS.sh" @@ -42,4 +43,5 @@ _consolidate_src() { - echo -e "\n####### $relpath #######" >> "$OUT" - sed '1{/^#!.*bash/d}' "$f" >> "$OUT" - echo -e "\n" >> "$OUT" + printf '\n####### %s #######\n' "$relpath" >> "$OUT" + sed '1{/^#!.*bash/d}' "$f" >> "$OUT" + printf '\n' >> "$OUT" done -echo "All library files have assembled in $LIB_ROOT/" + echo "All library files have been assembled in $LIB_ROOT/" }
50-81: Improve determinism and enforce naming constraints
- Sort
maprefkeys before iteration to guarantee consistent array output order.- (Optional) Validate that
parentconforms to Bash identifier rules to avoid invalid variable declarations.Example diff for key sorting:
@@ emit_section() { - for key in "${!mapref[@]}"; do + # Ensure consistent order of metadata entries + local keys=($(printf '%s\n' "${!mapref[@]}" | sort)) + for key in "${keys[@]}"; do
105-117: Enhance output reliability withprintf
Replace theecho "${array_entries[$arr]}"call withprintf '%s\n'to ensure embedded newlines and special characters are preserved exactly.@@ _write_arrays() { - echo "${array_entries[$arr]}" + printf '%s\n' "${array_entries[$arr]}"
119-139: Refine usage error handling & CLI UX
- Send the usage message to stderr when an invalid command is provided.
- Consider adding a
--help/-hflag for explicit usage guidance.Example diff:
@@ *) - echo "Usage: $0 [consolidate|generate|all]" + echo "Usage: $0 [consolidate|generate|all]" >&2 return 1
141-144: Remove or implement the unusedDEBUGvariable
TheDEBUGvariable is assigned but never used. Either drop it or leverage it (e.g., toggleset -x).@@ -142,2 +142,3 @@ if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then - DEBUG="${DEBUG:-}" + # Enable debug output if DEBUG is set + [[ -n "$DEBUG" ]] && set -x
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tools/10_consolidate_module.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/10_consolidate_module.sh
[warning] 98-98: section_kv appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (2)
tools/10_consolidate_module.sh (2)
5-11: Strict mode and variable initialization are solid.
The use ofset -euo pipefailensures robust failure handling, and initial vars (SRC_ROOT,OUT_FILE,LIB_ROOT,IGNORE_FILES) are clearly defined.
83-103: ShellCheck SC2034 false positive – skip
Thesection_kvarray is passed by reference intoemit_section, so it is used implicitly. No action needed.
renamed: tools/01_validate_module.sh -> tools/02_validate_module.sh renamed: tools/03_promote_module.sh -> tools/04_promote_module.sh renamed: tools/10_consolidate_module.sh -> tools/06_consolidate_module.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
tools/06_consolidate_module.sh (4)
1-14: Consider marking constants as readonly.The path and ignore-file variables (
SRC_ROOT,OUT_FILE,LIB_ROOT,IGNORE_FILES) are effectively constants. Declaring them asreadonlycan prevent accidental reassignment and improve clarity.
18-48: Preferprintfoverecho -eand document thesedtrick.Relying on
echo -eis less portable across shells. Usingprintfensures consistent interpretation of escape sequences. Also, thesed '1{/^#!.*bash/d}'command removes the shebang—consider adding an inline comment to explain its intent.Example diff:
-for f in "$DIR"/*.sh; do +for f in "$DIR"/*.sh; do [[ -e "$f" ]] || continue fname=$(basename "$f") skip=false for ignore in "${IGNORE_FILES[@]}"; do if [[ "${fname,,}" == "${ignore,,}" ]]; then skip=true break fi done if [[ "$skip" == true ]]; then continue fi relpath="${DIR%/}/$fname" - echo -e "\n####### $relpath #######" >> "$OUT" - sed '1{/^#!.*bash/d}' "$f" >> "$OUT" - echo -e "\n" >> "$OUT" + printf "\n####### %s #######\n" "$relpath" >> "$OUT" + # remove existing shebang from source + sed '1{/^#!.*bash/d}' "$f" >> "$OUT" + printf "\n" >> "$OUT" done
50-81: Escape values when emitting array entries.Embedding raw config values may break if they contain quotes or backslashes. Consider using
printf '%q'or another escaping mechanism to safely serialize eachmapref[$key].
105-117: Preferprintffor multi-line output in_write_arrays.Using
echo -ecan behave inconsistently;printfoffers clearer semantics:- echo -e "######## Auto-generated. Do not edit. ########\n" + printf "######## Auto-generated. Do not edit. ########\n\n"tools/04_promote_module.sh (1)
15-19: Handle empty globs or enablenullglob.If no
docs_*.mdfiles exist, the glob expands to the literal pattern. To avoid spurious iterations, either enableshopt -s nullglobat the top of the function or guard more strictly:+shopt -s nullglob for docs_file in ./staging/docs_*.md; do [ -f "$docs_file" ] || continue echo "Moving $docs_file to $DOC_ROOT/" mv "$docs_file" "$DOC_ROOT/" done +shopt -u nullglobtools/02_validate_module.sh (2)
4-20: Use<<-EOFto preserve indentation in help message.Switching to a here-doc with
<<-EOFallows the help text to be indented cleanly without injecting leading tabs:-_about_validate_module() { - cat <<EOF +_about_validate_module() { + cat <<-EOF
54-56: Consider anchoring field checks to avoid false positives.Current
_check_confusesgrep -qE "^$field=", which may match prefixes (feature_extra=). To avoid this, you can anchor the field name:-for field in "${REQUIRED_CONF_FIELDS[@]}"; do - if ! grep -qE "^$field=" "$file"; then +for field in "${REQUIRED_CONF_FIELDS[@]}"; do + if ! grep -qE "^${field}=" "$file"; then
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tools/00_setup_module.sh(1 hunks)tools/02_validate_module.sh(1 hunks)tools/04_promote_module.sh(1 hunks)tools/06_consolidate_module.sh(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tools/00_setup_module.sh
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/04_promote_module.sh
[warning] 10-10: array_entries appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 11-11: group_counts appears unused. Verify use (or export if used externally).
(SC2034)
tools/06_consolidate_module.sh
[warning] 98-98: section_kv appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (9)
tools/06_consolidate_module.sh (3)
50-81: Verify Bash version fordeclare -nnameref usage.The
emit_sectionfunction usesdeclare -nto create a nameref. That feature requires Bash ≥ 4.3. Please confirm that the target environment runs a compatible Bash version, or add a version check at the top of the script.
83-103: Skip false-positive ShellCheck warning forsection_kv.
section_kvis used via a Bash nameref (declare -n mapref), so ShellCheck’s SC2034 (“unused variable”) is a false positive here. No action needed.
119-139: Consolidation entry point is solid.The
consolidate_modulefunction cleanly handles subcommands and defaults toall. The case structure and direct-execution guard are correctly implemented.tools/04_promote_module.sh (3)
22-39: Script promotion logic is well-structured.The loop correctly skips non-matching files, extracts
parentfrom.conf, and moves both the script and its config atomically. Error messages for missingparentare clear.
43-45: Direct-execution guard is correct.Using
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]ensurespromote_moduleonly runs when the script is invoked directly.
10-11: Remove unused associative arrays.The declarations
declare -A array_entriesanddeclare -A group_countsare never referenced in this script. Removing them will reduce confusion and eliminate dead code.⛔ Skipped due to learnings
Learnt from: Tearran PR: Tearran/configng-v2#25 File: tools/03_promote_module.sh:10-11 Timestamp: 2025-06-17T20:56:24.174Z Learning: The arrays `array_entries` and `group_counts` declared in `tools/03_promote_module.sh` are actually used by `tools/10_consolidate_module.sh`, indicating these scripts work together in the module management workflow.tools/02_validate_module.sh (3)
22-38: Markdown validation logic is solid.The
_check_mdfunction correctly identifies missing files, missing headers, and header-only documents. The checks are clear and comprehensive.
40-52: Shell script check is accurate.Using
grepto detect the_about_<modulename>()function ensures each script includes its help stub. The pattern is correct under POSIX-basic regex.
95-98: Direct-execution guard is implemented correctly.The check ensures
validate_moduleonly runs when the script is called directly, which is standard practice.
modified: tools/04_promote_module.sh
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors and consolidates module management scripts by removing legacy scripts and introducing new ones with enhanced scaffolding, validation, promotion, and consolidation functionality. The changes improve the naming conventions, integrate debugging tests directly into the source, and streamline the module lifecycle.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/staging_03_promote_module.sh | Removed legacy promote script. |
| tools/staging_01_check_required.sh | Removed obsolete module file check script. |
| tools/staging_00_setup_scaffold.sh | Removed deprecated scaffold setup script. |
| tools/consolidate_01_lib_metadata.sh | Removed legacy metadata consolidation script. |
| tools/consolidate_00_lib_src.sh | Removed legacy library source consolidation script. |
| tools/06_consolidate_module.sh | Added new consolidation script combining source and configuration processing. |
| tools/04_promote_module.sh | Added updated promotion script moving files from staging to appropriate destinations. |
| tools/02_validate_module.sh | Added module validation with clear output statuses. |
| tools/00_setup_module.sh | Added updated module scaffold setup script with integrated metadata, module, and documentation generation. |
| tests/test_debug.sh | Removed outdated standalone debug tests in favor of embedded debug logic in the initialize script. |
| src/initialize/debug.sh | Updated debug script with improved help message formatting and inline testing logic. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tools/04_promote_module.sh (1)
12-12: Update misleading commentThe comment refers to
docs_*.shalthough documentation files are.md. Update it to accurately reflect the action.Example:
-# Move *.sh (not docs_*.sh) with a matching .conf file to src/$parent/ +# Move staging .sh files with a matching .conf file to src/$parent/
🧹 Nitpick comments (1)
tools/04_promote_module.sh (1)
13-29: Scopeparentvariable locally
parentis assigned inside the loop but not declared local, which can leak its value outside the loop and cause cross-contamination between loops. Declare it as local.Example:
-for sh_file in ./staging/*.sh; do +for sh_file in ./staging/*.sh; do [ -f "$sh_file" ] || continue base_name=$(basename "$sh_file" .sh) conf_file="./staging/${base_name}.conf" if [ -f "$conf_file" ]; then - parent=$(grep '^parent=' "$conf_file" | head -n1 | cut -d= -f2- | xargs) + local parent + parent=$(grep '^parent=' "$conf_file" | head -n1 | cut -d= -f2- | xargs) if [ -n "$parent" ]; then dest_dir="./src/$parent" mkdir -p "$dest_dir"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tools/00_setup_module.sh(1 hunks)tools/04_promote_module.sh(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tools/00_setup_module.sh
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/04_promote_module.sh
[warning] 9-9: array_entries appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 10-10: group_counts appears unused. Verify use (or export if used externally).
(SC2034)
This pull request introduces several improvements and additions to the module management scripts, including the creation of new tools, consolidation of functionality, and refactoring of existing scripts. The changes aim to streamline the module lifecycle, from scaffolding and validation to promotion and consolidation, while removing redundant or outdated scripts.
New Features and Additions:
tools/00_setup_module.shto create scaffolding for new modules, including.conf,.sh, and.mdfiles in the./stagingdirectory. This replaces the olderstaging_00_setup_scaffold.shscript.tools/01_validate_module.shto validate the presence and completeness of module files (.conf,.sh, and.md) in the./stagingdirectory.tools/03_promote_module.shto move validated files from./stagingto their appropriate directories (./docsand./src/<parent>).tools/10_consolidate_module.shto consolidate.shfiles into a single library and generate options arrays from.conffiles. This combines and replaces functionality fromconsolidate_00_lib_src.shandconsolidate_01_lib_metadata.sh.Refactoring and Cleanup:
staging_00_setup_scaffold.sh,staging_01_check_required.sh,staging_03_promote_module.sh,consolidate_00_lib_src.sh, andconsolidate_01_lib_metadata.sh) as their functionality has been replaced by the new tools. [1] [2] [3] [4] [5]src/initialize/debug.sh, removing the need for a separatetests/test_debug.shscript. [1] [2]Minor Improvements:
debug.shfor consistency by removing the use of tabs.Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests