docs+feat: address pre-v0.1.0 release review findings#383
Conversation
README
- Fix runtime.prepare example: the function is 2-arg (placeholder
dialect is baked into RawQuery), not 3-arg. README snippet now
matches src/sqlode/runtime.gleam:72. Add a README snapshot test
that reads README.md and asserts the 2-arg call shape, so
regressions are caught by `gleam test`.
- Elevate the opaque-types warning to a callout at the top of
"Custom type aliases". The detail paragraph stays underneath.
- New top-level "Limitations" section with three subsections users
are likely to hit before the CLI explains them:
- parameter type inference (INSERT / comparison / IN / cast)
- schema DDL scope (snapshot or add-only, never drop/rename)
- view resolution + strict_views opt-in
- Inline a short DDL-scope note next to the existing directory
ingestion sentence so readers see it before they feed a migration
directory in.
strict_views config option
- Add `strict_views: Bool` to `model.GleamOutput`, default False
so existing configs behave unchanged.
- Accept `strict_views` in `config.gleam`'s gleam-node whitelist
and parse it with the same optional-bool helper as vendor_runtime.
- In generate.gleam's load_catalog, if strict_views is True and
schema_parser emits any SchemaWarning, return SchemaParseError
with a concatenated detail instead of printing to stderr and
continuing. The legacy stderr path stays for strict_views: False.
Tests
- test/generate_test.gleam: strict_views True -> SchemaParseError
whose detail mentions `strict_views` and the skipped column name;
strict_views False -> generation still succeeds. Uses new fixtures
`test/fixtures/strict_views_schema.sql` and `strict_views_query.sql`.
- test/config_test.gleam: strict_views defaults to False, and
strict_views: true in YAML round-trips to the field. New fixture
`test/fixtures/strict_views_enabled.yaml`.
- Thread strict_views: False through every existing GleamOutput
construction site in tests (no behaviour change).
Post-release follow-up tracked separately as #382
(parser/analyzer tokenised-query IR refactor).
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 21 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1 participant
Summary
Addresses six findings from the pre-v0.1.0 release review. All README-only fixes + one new opt-in config flag (
strict_views). The parser IR refactor (finding #2) is intentionally deferred — tracked as #382 for post-release.Changes
README
runtime.prepareexample: the function is 2-arg (placeholder dialect is baked intoRawQuery), not 3-arg. README previously produced an immediate compile error on copy-paste.test/codegen_test.gleam::readme_runtime_prepare_is_two_arg_test) that readsREADME.mdand asserts the 2-arg call shape so the example cannot silently regress.[!WARNING]callout at the top of "Custom type aliases". The detailed explanation stays underneath.## Limitationssection with three subsections users hit in practice:DROP/RENAME/ALTER COLUMN)strict_viewsopt-instrict_views config option
strict_views: Boolfield onmodel.GleamOutput, defaultFalseso existing configs are unaffected.config.gleamgleam-node whitelist and parsed with the same optional-bool helper asvendor_runtime.generate.gleam'sload_catalog, ifstrict_views: Trueandschema_parserproduces anySchemaWarning, returnSchemaParseErrorwith a concatenated detail instead of printing to stderr and continuing. The legacy stderr-warn-and-continue path remains forstrict_views: False.Tests
test/generate_test.gleam: strict mode rejects unresolvable view columns (detail containsstrict_views+ the skipped column name); legacy mode still succeeds. New fixturestest/fixtures/strict_views_schema.sql+strict_views_query.sql.test/config_test.gleam:strict_viewsdefaults toFalse; YAMLstrict_views: trueround-trips correctly. New fixturetest/fixtures/strict_views_enabled.yaml.GleamOutputconstructor call-site in tests now passesstrict_views: False— mechanical, no behaviour change.Design Decisions
Boolfield, not aStrictModerecord. Matches the existingomit_unused_models/vendor_runtimeprecedent. If more strict knobs appear later they can be grouped then.generate.gleam, not inschema_parser.schema_parseralready distinguishesParseError(fatal) fromSchemaWarning(soft). Threading a flag into the parser would have rippled into every caller that consumesparse_files_with_engine; instead the decision stays at the generate layer where the config is naturally available.SchemaParseErrorvariant with a concatenated detail rather than introducing a newStrictViewsFailederror. Keepserror_to_stringand CLI output untouched.Limitations
test/schema_parser_test.gleam:view_nonexistent_table_teststill asserts the silent-drop behaviour intentionally — it documents the default (non-strict) path.Verification
Local:
just all→ 833 unit tests pass, 20 shellspec examples pass (gleam format check + gleam check + gleam build --warnings-as-errors)sh integration_test/run.sh→ all 7 integration cases passCloses #0 (pre-release review findings, no explicit issue)
Refs #382 (parser IR follow-up)