Name the Postgres flag after Postgres, and start MySQL in deploy-full - #62
Merged
Merged
Conversation
--with-database was named when Postgres was the only target for the database secrets engine, so it read as "the database feature" rather than as one of two engines. --with-mysql then arrived named after its engine, and the pair has been asymmetric since: nothing in the two names tells a reader that one starts Postgres and the other MySQL, and the flag that sounds like it covers both covers neither more than the other. --with-database stays as an alias, warning rather than failing. It appears in every published example of this repository up to now, including ones a reader may already have copied into their own CI, and breaking those buys nothing but a shorter case statement. The warning goes to stderr like every other log line, so ROOT_TOKEN=$(...) still captures only the token. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
deploy-full described itself as every optional service, as the integration suite runs it, and then did not pass --with-mysql while the suite did. Anyone reproducing an integration failure locally got a cluster with one database engine instead of two, and nothing said so — the MySQL assertions simply had no target, which is the shape of failure this repository exists to catch rather than produce. The description is corrected at the same time. The target also passes --with-oidc, which the integration suite does not (human login has its own CI job), so it is a superset of that suite and not a match for it. The Makefile header asks targets to say where they diverge from CI, and this one was quietly claiming the opposite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
CI is green on all 27 jobs, which settles the three checks the
Human OIDC login (Dex) also passes, which is the job the new Makefile Still not covered, unchanged from the description: nothing fails if a |
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.
--with-databasewas named when Postgres was the only target for thedatabase secrets engine, so it read as "the database feature" rather than
as one of two engines.
--with-mysqlthen arrived named after itsengine, and the pair has been asymmetric since — nothing in the two names
tells a reader which database each one starts.
Two commits, because the second is a bug that predates the first.
Name the Postgres flag after Postgres
--with-postgresis the name;--with-databasestays as an alias thatwarns rather than fails. The old spelling appears in every published
example of this repository up to now, including ones a reader may already
have copied into their own CI, and breaking those buys nothing but a
shorter case statement. The warning goes to stderr like every other log
line, so
ROOT_TOKEN=$(...)still captures only the token.Start the second database engine in deploy-full
make deploy-fulldescribed itself as every optional service as theintegration suite runs it, then did not pass
--with-mysqlwhile thesuite did. Anyone reproducing an integration failure locally got a
cluster with one database engine instead of two, and nothing said so —
the MySQL assertions simply had no target.
The description is corrected at the same time: the target also passes
--with-oidc, which the suite does not (human login has its own CI job),so it is a superset of that suite rather than a match for it. The
Makefile header asks targets to say where they diverge from CI, and this
one was quietly claiming the opposite.
Checked
--with-*flag used anywhere inMakefile,README.md,CLAUDE.md,docs/,tests/,.github/anddocker/is accepted bythe parser; no caller passes a flag the script would reject.
WITH_POSTGRES=true; the alias warns onstderr only; unknown flags still exit 1.
deploy-full's exact flag list enables all six optional services, andthe integration suite's exact list enables the same five minus OIDC —
so superset is measured, not assumed.
make helprenders the new rowcorrectly (the added comment block does not leak into it).
bash -nclean;tests/lint(1/1) andtests/docs-index(7/7) pass;exec bits still
100755.Not covered
Nothing fails if a future edit drops the
--with-databasealias — theintegration suite now uses the new spelling only, and there is no shim
harness for
bootstrap-dev-cluster.shto hang an assertion on. The aliasis best-effort, not load-bearing.
shellcheck, markdownlint and
makeitself were not available in theauthoring shell, so CI is their first run here; the integration suite is
the first thing that actually starts MySQL by way of
deploy-full.🤖 Generated with Claude Code