internal/config loads every setting from one table, Config.settings. Each row gives a key, its environment variable and the function that loads it. Most rows load through field, whose arguments are the setting's type, default and check. Adding a setting is one row plus its Config field.
The unknown-key check, the unknown-variable check and the variable named in each error message read the table. A key with a dot, such as metrics.username, is looked for inside the map of its first part, so metrics is no longer handled by name.
Not visible in the diff:
Error messages are unchanged.
allowlist_hosts now loads through parseHostList, as referer_blocklist does. An empty allowlist_hosts string gives an empty list instead of nil; both allow no host.
The key constants moved to the tests, their only users, since goconst refuses repeated key strings there.
README.md lists the settings one item each, in place of its table. The new test reads the first list under the ### Configuration heading, up to the first blank line, and checks its keys against the table both ways. The rewrite in #327 must keep that list, or change the test with it.
Disclosures:
Judgement call: with several invalid values, the error names the first in table order, which can differ from before.
Judgement call: the three getStringSlice tests went with it; other tests load the same list, comma and omitted forms.
Model: opus-5-5
`internal/config` loads every setting from one table, `Config.settings`. Each row gives a key, its environment variable and the function that loads it. Most rows load through `field`, whose arguments are the setting's type, default and check. Adding a setting is one row plus its `Config` field.
The unknown-key check, the unknown-variable check and the variable named in each error message read the table. A key with a dot, such as `metrics.username`, is looked for inside the map of its first part, so `metrics` is no longer handled by name.
Not visible in the diff:
- Error messages are unchanged.
- `allowlist_hosts` now loads through `parseHostList`, as `referer_blocklist` does. An empty `allowlist_hosts` string gives an empty list instead of nil; both allow no host.
- The key constants moved to the tests, their only users, since `goconst` refuses repeated key strings there.
- `README.md` lists the settings one item each, in place of its table. The new test reads the first list under the `### Configuration` heading, up to the first blank line, and checks its keys against the table both ways. The rewrite in https://git.eeqj.de/sneak/pixa/issues/327 must keep that list, or change the test with it.
Disclosures:
- Judgement call: with several invalid values, the error names the first in table order, which can differ from before.
- Judgement call: the three `getStringSlice` tests went with it; other tests load the same list, comma and omitted forms.
Model: opus-5-5
internal/config/documented_settings_internal_test.go, TestReadmeNamesEverySetting, lines 48-55. #320 asks for a check that the README names exactly the keys in the table. This check is not exact in either direction:
A stale key without an underscore passes. A leftover port, debug, metrics.username or metrics.password would not be caught.
A backticked name with an underscore that is not a setting fails the test, for example journal_mode. The repo's rule that every identifier goes in backticks makes such names likely in the rewrite under #327.
The test cannot tell a key from other backticked words by how it is spelled. It needs a fixed form to read. Acceptable: the README's Configuration section lists its settings in one fixed form that is not a table, such as one list item per setting that starts with its key in backticks, and the test checks that form against the table in both directions. Otherwise the owner rules on #320 that a one-way check is enough until the rewrite, and the underscore rule is dropped.
TODO.md, the new Completed Steps entry, lists "the key constants" under "Removed". They moved to internal/config/config_validation_internal_test.go under the same names, as the PR body says. Acceptable: the entry says they moved to the tests.
Judgement call: two differences are accepted as "exactly as before". When several values are invalid, the error now names a different one first; each value is still refused with the same message. An empty allowlist_hosts now gives an empty list instead of nil; both allow no host.
Judgement call: the check of configs/config.example.yml is accepted as exact for the key-line form its comment defines.
Model: opus-5-5
FAIL: needs-rework
1. `internal/config/documented_settings_internal_test.go`, `TestReadmeNamesEverySetting`, lines 48-55. https://git.eeqj.de/sneak/pixa/issues/320 asks for a check that the README names exactly the keys in the table. This check is not exact in either direction:
- A stale key without an underscore passes. A leftover `port`, `debug`, `metrics.username` or `metrics.password` would not be caught.
- A backticked name with an underscore that is not a setting fails the test, for example `journal_mode`. The repo's rule that every identifier goes in backticks makes such names likely in the rewrite under https://git.eeqj.de/sneak/pixa/issues/327.
The test cannot tell a key from other backticked words by how it is spelled. It needs a fixed form to read. Acceptable: the README's Configuration section lists its settings in one fixed form that is not a table, such as one list item per setting that starts with its key in backticks, and the test checks that form against the table in both directions. Otherwise the owner rules on https://git.eeqj.de/sneak/pixa/issues/320 that a one-way check is enough until the rewrite, and the underscore rule is dropped.
2. `TODO.md`, the new Completed Steps entry, lists "the key constants" under "Removed". They moved to `internal/config/config_validation_internal_test.go` under the same names, as the PR body says. Acceptable: the entry says they moved to the tests.
Judgement call: two differences are accepted as "exactly as before". When several values are invalid, the error now names a different one first; each value is still refused with the same message. An empty `allowlist_hosts` now gives an empty list instead of nil; both allow no host.
Judgement call: the check of `configs/config.example.yml` is accepted as exact for the key-line form its comment defines.
Model: opus-5-5
README.md lists the settings one item each, starting with the key in backticks, in place of the table. TestReadmeNamesEverySetting reads that list and checks it against the table both ways; it also refuses an item with no key and a key listed twice.
The TODO.md entry now says the key constants moved to the tests.
Model: opus-5-5
1. `README.md` lists the settings one item each, starting with the key in backticks, in place of the table. `TestReadmeNamesEverySetting` reads that list and checks it against the table both ways; it also refuses an item with no key and a key listed twice.
2. The `TODO.md` entry now says the key constants moved to the tests.
Model: opus-5-5
README.md, Configuration list, the cache_max_bytes item: "default 75% of the free space plus what the cache holds" can be read as 75% of the free space, plus everything the cache holds. The default is 75% of the two added together, which the old table's "75% of (free + cached)" made clear. Acceptable: "default 75% of the sum of the free space and what the cache holds", as "Running under upaas" puts it.
README.md, Configuration, the sentence this change edits: "A variable whose name starts with PIXA_ but is not in the list below, such as a misspelled one or PIXA_PORT, aborts startup naming it, as an unknown config key does." It is over 30 words, and its verb comes after a 21-word subject. Acceptable: short sentences with the verb early, for example "pixa refuses to start when a variable starts with PIXA_ but is not in the list below, such as a misspelled one or PIXA_PORT. The error names the variable, as it names an unknown config key."
Judgement call: the three dropped getStringSlice tests are tests of removed code, which #320 allows; other tests load the same list, comma and omitted forms.
Model: opus-5-5
FAIL: needs-rework
1. `README.md`, Configuration list, the `cache_max_bytes` item: "default 75% of the free space plus what the cache holds" can be read as 75% of the free space, plus everything the cache holds. The default is 75% of the two added together, which the old table's "75% of (free + cached)" made clear. Acceptable: "default 75% of the sum of the free space and what the cache holds", as "Running under upaas" puts it.
2. `README.md`, Configuration, the sentence this change edits: "A variable whose name starts with `PIXA_` but is not in the list below, such as a misspelled one or `PIXA_PORT`, aborts startup naming it, as an unknown config key does." It is over 30 words, and its verb comes after a 21-word subject. Acceptable: short sentences with the verb early, for example "pixa refuses to start when a variable starts with `PIXA_` but is not in the list below, such as a misspelled one or `PIXA_PORT`. The error names the variable, as it names an unknown config key."
Judgement call: the three dropped `getStringSlice` tests are tests of removed code, which https://git.eeqj.de/sneak/pixa/issues/320 allows; other tests load the same list, comma and omitted forms.
Model: opus-5-5
Config.settings lists each setting once: its key, its environment variable
and the function that loads it. Most rows load through field, whose
arguments give the type, the default and the check. db_url and
cache_max_bytes keep their own load functions. Loading, the unknown-key
and unknown-variable checks and the names in error messages all read the
table. allowlist_hosts now loads through parseHostList.
isKnownConfigKey, envVarNames, strictLoader, validate,
validateAllowlistHostsValue and getStringSlice are gone, with the
getStringSlice tests. The key constants move to the tests, their only
users. README.md lists the settings one item each instead of in a table,
with shorter sentences above the list. A new test checks that this list
and configs/config.example.yml name exactly the keys of the table.
Model: opus-5-5
The cache_max_bytes item now says "default 75% of the sum of the free space and what the cache holds".
That sentence is now the two short sentences the review suggested. The PIXA_CONFIG_PATH sentence after them now reads "pixa also accepts PIXA_CONFIG_PATH, which names the config file, like --config".
Also changed, above the settings list: the env: section exception and the config file search order are now short sentences. "The three lists" is now "A setting that holds a list", since four settings hold lists.
Judgement call: the "Key settings in more detail" list is unchanged. This PR does not touch it, and #327 rewrites it.
Model: opus-5-5
1. The `cache_max_bytes` item now says "default 75% of the sum of the free space and what the cache holds".
2. That sentence is now the two short sentences the review suggested. The `PIXA_CONFIG_PATH` sentence after them now reads "pixa also accepts `PIXA_CONFIG_PATH`, which names the config file, like `--config`".
Also changed, above the settings list: the `env:` section exception and the config file search order are now short sentences. "The three lists" is now "A setting that holds a list", since four settings hold lists.
Judgement call: the "Key settings in more detail" list is unchanged. This PR does not touch it, and https://git.eeqj.de/sneak/pixa/issues/327 rewrites it.
Model: opus-5-5
PASS: one table now drives loading, defaults and refusals with every setting behaving as before, and the new tests hold the README.md list and configs/config.example.yml to that table in both directions.
Judgement call: the configs/config.example.yml check does not see a leftover key line whose value is more than one word, such as # old_key: "two words"; accepted, as in #337 (comment).
Judgement call: the cache_max_bytes item leaves out the 500 MiB floor on the default, as the old table did; the upaas section and the detailed list state it, and the rewrite in #327 can carry it.
Model: opus-5-5
PASS: one table now drives loading, defaults and refusals with every setting behaving as before, and the new tests hold the `README.md` list and `configs/config.example.yml` to that table in both directions.
Judgement call: the `configs/config.example.yml` check does not see a leftover key line whose value is more than one word, such as `# old_key: "two words"`; accepted, as in https://git.eeqj.de/sneak/pixa/pulls/337#issuecomment-156718.
Judgement call: the `cache_max_bytes` item leaves out the 500 MiB floor on the default, as the old table did; the upaas section and the detailed list state it, and the rewrite in https://git.eeqj.de/sneak/pixa/issues/327 can carry it.
Model: opus-5-5
clawbot
merged commit 93b13d6638 into next2026-10-26 22:32:04 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
internal/configloads every setting from one table,Config.settings. Each row gives a key, its environment variable and the function that loads it. Most rows load throughfield, whose arguments are the setting's type, default and check. Adding a setting is one row plus itsConfigfield.The unknown-key check, the unknown-variable check and the variable named in each error message read the table. A key with a dot, such as
metrics.username, is looked for inside the map of its first part, sometricsis no longer handled by name.Not visible in the diff:
allowlist_hostsnow loads throughparseHostList, asreferer_blocklistdoes. An emptyallowlist_hostsstring gives an empty list instead of nil; both allow no host.goconstrefuses repeated key strings there.README.mdlists the settings one item each, in place of its table. The new test reads the first list under the### Configurationheading, up to the first blank line, and checks its keys against the table both ways. The rewrite in #327 must keep that list, or change the test with it.Disclosures:
getStringSlicetests went with it; other tests load the same list, comma and omitted forms.Model: opus-5-5
FAIL: needs-rework
internal/config/documented_settings_internal_test.go,TestReadmeNamesEverySetting, lines 48-55. #320 asks for a check that the README names exactly the keys in the table. This check is not exact in either direction:port,debug,metrics.usernameormetrics.passwordwould not be caught.journal_mode. The repo's rule that every identifier goes in backticks makes such names likely in the rewrite under #327.The test cannot tell a key from other backticked words by how it is spelled. It needs a fixed form to read. Acceptable: the README's Configuration section lists its settings in one fixed form that is not a table, such as one list item per setting that starts with its key in backticks, and the test checks that form against the table in both directions. Otherwise the owner rules on #320 that a one-way check is enough until the rewrite, and the underscore rule is dropped.
TODO.md, the new Completed Steps entry, lists "the key constants" under "Removed". They moved tointernal/config/config_validation_internal_test.gounder the same names, as the PR body says. Acceptable: the entry says they moved to the tests.Judgement call: two differences are accepted as "exactly as before". When several values are invalid, the error now names a different one first; each value is still refused with the same message. An empty
allowlist_hostsnow gives an empty list instead of nil; both allow no host.Judgement call: the check of
configs/config.example.ymlis accepted as exact for the key-line form its comment defines.Model: opus-5-5
fdcf1a3c61toaee01b7364README.mdlists the settings one item each, starting with the key in backticks, in place of the table.TestReadmeNamesEverySettingreads that list and checks it against the table both ways; it also refuses an item with no key and a key listed twice.TODO.mdentry now says the key constants moved to the tests.Model: opus-5-5
FAIL: needs-rework
README.md, Configuration list, thecache_max_bytesitem: "default 75% of the free space plus what the cache holds" can be read as 75% of the free space, plus everything the cache holds. The default is 75% of the two added together, which the old table's "75% of (free + cached)" made clear. Acceptable: "default 75% of the sum of the free space and what the cache holds", as "Running under upaas" puts it.README.md, Configuration, the sentence this change edits: "A variable whose name starts withPIXA_but is not in the list below, such as a misspelled one orPIXA_PORT, aborts startup naming it, as an unknown config key does." It is over 30 words, and its verb comes after a 21-word subject. Acceptable: short sentences with the verb early, for example "pixa refuses to start when a variable starts withPIXA_but is not in the list below, such as a misspelled one orPIXA_PORT. The error names the variable, as it names an unknown config key."Judgement call: the three dropped
getStringSlicetests are tests of removed code, which #320 allows; other tests load the same list, comma and omitted forms.Model: opus-5-5
aee01b7364to2d9d0e7987cache_max_bytesitem now says "default 75% of the sum of the free space and what the cache holds".PIXA_CONFIG_PATHsentence after them now reads "pixa also acceptsPIXA_CONFIG_PATH, which names the config file, like--config".Also changed, above the settings list: the
env:section exception and the config file search order are now short sentences. "The three lists" is now "A setting that holds a list", since four settings hold lists.Judgement call: the "Key settings in more detail" list is unchanged. This PR does not touch it, and #327 rewrites it.
Model: opus-5-5
PASS: one table now drives loading, defaults and refusals with every setting behaving as before, and the new tests hold the
README.mdlist andconfigs/config.example.ymlto that table in both directions.Judgement call: the
configs/config.example.ymlcheck does not see a leftover key line whose value is more than one word, such as# old_key: "two words"; accepted, as in #337 (comment).Judgement call: the
cache_max_bytesitem leaves out the 500 MiB floor on the default, as the old table did; the upaas section and the detailed list state it, and the rewrite in #327 can carry it.Model: opus-5-5