Uh oh!
There was an error while loading. Please reload this page.
refactor(src): move module aggregators inside their directory as index.sh - #934
Merged
Merged
Conversation
…x.sh `src/runner.sh` -> `src/runner/index.sh`, `src/coverage.sh` -> `src/coverage/index.sh`. A module is now one self-contained directory whose entry point is where every other ecosystem puts it (index.ts, __init__.py, mod.rs). Amends ADR-010, which chose "beside" two days ago. The reasoning that first chose "beside" was wrong. It rested on matching the `src/assertions.sh` -> `src/assert_*.sh` precedent, but assertions.sh aggregates *flat* files and has no directory: it is not a module aggregator, it is unaffected by this choice, and the "two conventions in src/" risk it was meant to avoid never existed. What actually decides it is a correctness bug. The rule that aggregators hold only `source` lines is enforced by test_module_aggregators_hold_only_source_lines_and_comments, which named them in a hand-maintained string: "src/assertions.sh src/runner.sh". #928 added src/coverage.sh and never appended it, so the rule silently stopped covering that file within one module of being introduced. With the entry point at a predictable src/*/index.sh the test discovers modules by glob and cannot drift; this commit converts it, and adds a test asserting the discovery finds every module. Verified by appending a statement to src/coverage/index.sh -- previously green, now red. Doing it now costs two renames. #931 adds nine more modules, after which it would cost eleven. The distributable is unchanged: diffing the built artifact before and after shows only the two renamed embed markers and two reworded header comments, same 17627 lines. `bash build.sh bin -v` prints "Build verified". Also corrects two references #928 left stale: the branch-helper dynamic-scope note in bash-style.md pointed at `src/coverage.sh:818-821`, and the unreleased CHANGELOG entry described a shape that no longer ships. Related #931
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #931
ADR-010 put each module's aggregator beside its directory (
src/runner.sh+src/runner/*.sh). This amends that decision: the entry point moves inside asindex.sh, matching what most ecosystems do (index.ts,__init__.py,mod.rs).The original reasoning was wrong. It rested on matching the
src/assertions.sh→src/assert_*.shprecedent — butassertions.shaggregates flat files and has no directory. It is not a module aggregator, it is unaffected either way, and the "two conventions insrc/" risk it was meant to avoid never existed.💡 Changes
src/runner.sh→src/runner/index.sh,src/coverage.sh→src/coverage/index.sh; git records both as pure renames (1 line changed each — a reworded header comment).sourcelines" rule was enforced against a hand-maintained list,"src/assertions.sh src/runner.sh". refactor(coverage): split src/coverage.sh into a src/coverage/ module #928 addedsrc/coverage.shand never appended it, so the rule silently stopped covering that file within one module of being introduced. With the entry point at a predictablesrc/*/index.sh, the test now discovers modules by glob and cannot drift — plus a test asserting the discovery finds every module.bash-style.mdpointed atsrc/coverage.sh:818-821, and the unreleased CHANGELOG entry described a shape that no longer ships.Doing this now costs two renames. #931 adds nine more modules, after which it costs eleven.
✅ Verification
The distributable is unchanged. Diffing the built artifact before and after shows only the two renamed embed markers and the two reworded header comments — same 17627 lines.
The glob test was confirmed to catch what the hardcoded list could not: appending a statement to
src/coverage/index.shwas previously green, now red.Green: sequential ·
--parallel·--parallel --simple --strict·make sa·make lint· CI-mode ShellCheck (per file, no-x) ·bash build.sh bin -v→✅ Build verified ✅.