refactor: name things once, and say the rest in code - #17
Merged
Merged
Conversation
added 2 commits
September 12, 2026 23:40
`docker compose --profile migrate config --services | grep -qx migrate` looks correct and fails about one run in seven. grep -q exits on its first match and closes the pipe; docker compose is still writing, dies of SIGPIPE, and this script's own `set -o pipefail` reports the whole pipeline as failed. The effect on a client is a stack that refuses to migrate, at random, with "a database service exists but no migrate service was found β refusing to start with unapplied schema" β about a service that is sitting right there. Measured 6 failures in 40 runs against a generated nextjs+nestjs+postgres project; 0 in 40 after. compose_has_service captures the list first and matches against it, so nothing closes a pipe early. The test stub writes past the match by more than one pipe buffer, which makes the old shape fail deterministically.
No behaviour change. Everything below is naming, shape and comments. Headers: one format on every script, giving the file, what it does and the author; executables also carry a usage and an example. Vocabulary: one verb per idea, so a reader can guess. assert_ dies on a bad value, require_ is an environment precondition, resolve_ works a value out, record_ writes it down, load_ reads a definition into variables, print_ only prints, and a bare noun is a getter. Twelve functions were renamed to fit, three of which said the wrong thing outright: resolve_minimum_release_age -> record_release_age_exceptions verify_workspace_filter_name -> assert_workspace_filter_name resolve_workspace_filter_name -> substitute_workspace_filter The last two were near-identical names for opposite actions. Shape: long functions became named steps. cmd_new went from 143 lines to 27, and cmd_add, cmd_update, cmd_publish, lint_adapters, lint_services and apply_service_drivers followed. scripts/deploy-check.sh and scripts/check-provenance.sh were straight-line scripts and are now a main calling named steps. 126 functions became 187; the longest went from 143 lines to 69. Duplication: merge_compose_fragment replaces four copies of the same yq-merge-or-die block, adapter_env_value four copies of the same sed, relax_pnpm_workspace/restore_pnpm_workspace three copies of the same three-line sed, resolve_project two copies of a ten-line block. Constants: FIRST_APP_PORT and APP_CONTAINER_PORT (which removed the magic 8079 and the paragraph explaining it), PNPM_RELAXATIONS, PUBLISH_UNSUPPORTED, SHARED_DRIVERS_DIR, PasswordPlaceholder, ESC_SEQUENCE_TIMEOUT and others. Comments: 18,505 words to 13,166, 36% of lines to 27%. Fourteen were pure signature echoes and are gone; the rest were compressed. What remains is third-party landmines β mise exec trusting a parent config, pnpm turning on frozen lockfiles under CI, yq collapsing a document without -P β which is why the density stays above immich's 14%. Not done: readonly on the constants. These libraries are re-sourced into child processes by design, and a second readonly is an error that set -e turns into a dead script.
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 free
to 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.
What this changes
Two things, in two commits.
fix(install)β a real defect this pass turned up.common/install.shdecided whether a stack had a migrate service with
docker compose --profile migrate config --services | grep -qx migrategrep -qexits on its first match and closes the pipe;docker composethendies of SIGPIPE, and
set -o pipefailreports the whole pipeline as failed.Measured 6 failures in 40 runs against a real generated stack. The effect on
a client: roughly one
install.shrun in seven refuses to migrate with"a database service exists but no migrate service was found β refusing to start
with unapplied schema", about a service that is sitting right there. This ships
into every generated project.
compose_has_servicecaptures the list first andmatches against it β 0 failures in 40 after.
refactorβ the rest. No behaviour change intended anywhere else.assert_dies on a bad value (9),require_is anenvironment precondition (3),
resolve_works a value out (5),record_writes it down (4),
load_reads a definition into variables (4),print_only prints (3), a bare noun is a getter. Twelve functions were renamed to fit
it, including three whose names said the wrong thing:
resolve_minimum_release_ageβrecord_release_age_exceptions(it records,it never resolved), and the near-identical
verify_workspace_filter_name/resolve_workspace_filter_nameβassert_workspace_filter_name/substitute_workspace_filter.cmd_new143 β 27 lines, and the samefor
cmd_add,cmd_update,cmd_publish,lint_adapters,lint_services,apply_service_drivers.scripts/deploy-check.shandscripts/check-provenance.shwere straight-line scripts; both are now amaincalling named steps. 126 β 187 functions, longest 143 β 69 lines.merge_compose_fragmentreplaces fourcopies of the same yq-merge-or-die block,
adapter_env_valuefour copies ofthe same sed,
relax_pnpm_workspace/restore_pnpm_workspacethree copies ofthe same three-line sed,
resolve_projecttwo copies of a ten-line block.FIRST_APP_PORT/APP_CONTAINER_PORT(which removed the magic
8079and the paragraph explaining it),PNPM_RELAXATIONS,PUBLISH_UNSUPPORTED,SHARED_DRIVERS_DIR,PasswordPlaceholder,ESC_SEQUENCE_TIMEOUT, and others.36% β 27% of lines. Fourteen were pure signature echoes and are gone; the rest
were compressed, not deleted β what is left is third-party landmines
(
mise exectrusting a parent config, pnpm turning on frozen lockfiles underCI, yq collapsing a document without
-P).lib/manifest.sh; fixed.Not done, deliberately:
readonlyon the constants. These libraries arere-sourced into child processes by design, and a second
readonlyis an errorthat
set -eturns into a dead script. immich usesreadonlyin none of itsshell scripts either.
How it was verified
mise run lintβ clean.mise run test-runnerβ both lanes under the runner's own environment.mise exec -- bats tests/provenance.batsβ 4/4.mise exec -- zizmor --min-severity medium .github/workflows/β no findings../scripts/deploy-check.sh nextjs nestjs --db postgresagainst real Docker βtwo images built, both containers healthy, liveness 200, readiness 200,
migrations applied. Run three times over this branch; the one red run is what
found the SIGPIPE bug above.
at 0/40, against a generated nextjs+nestjs+postgres project.
renamed, the rest are new splits.
Checklist
mise run lintpassesmise run test-runnerpasses