Conversation
Reviewer's GuideRefactors test schemas out of test_helper into dedicated modules under lib/test_schema, tweaks CI logging for Typesense health checks, and makes minor documentation and changelog updates including noting support for Typesense v28.0. Class diagram for refactored test_schema modulesclassDiagram
class TestHelper {
}
class TestSchemaCar {
}
class TestSchemaCatalog {
}
class TestSchemaCredential {
}
class TestSchemaHouse {
}
class TestSchemaMultiSearch {
}
class TestSchemaPerson {
}
class TestSchemaProduct {
}
class TestSchemaTruck {
}
TestHelper ..> TestSchemaCar
TestHelper ..> TestSchemaCatalog
TestHelper ..> TestSchemaCredential
TestHelper ..> TestSchemaHouse
TestHelper ..> TestSchemaMultiSearch
TestHelper ..> TestSchemaPerson
TestHelper ..> TestSchemaProduct
TestHelper ..> TestSchemaTruck
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
ExTypesense.TestSchema.Productencoder still callsMap.take/2with:poducts_idbut only rewrites:products_id, which means theidfield will never be copied as intended; consider fixing the atom name so the virtual id field is populated correctly. - All the
ExTypesense.TestSchema.*modules have been moved fromtest/test_helper.exsintolib/test_schema, which means these test-only schemas will now ship as part of the library code; consider relocating them under a test-support path (e.g.test/support) or otherwise scoping them to the test environment. - In the README v2 warning block, "The Purpose of v2" is capitalized mid‑sentence and the sentence about
optsnow reads a bit awkwardly split across lines; consider reverting to lower‑case "purpose" and tightening the line break to read more naturally.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `ExTypesense.TestSchema.Product` encoder still calls `Map.take/2` with `:poducts_id` but only rewrites `:products_id`, which means the `id` field will never be copied as intended; consider fixing the atom name so the virtual id field is populated correctly.
- All the `ExTypesense.TestSchema.*` modules have been moved from `test/test_helper.exs` into `lib/test_schema`, which means these test-only schemas will now ship as part of the library code; consider relocating them under a test-support path (e.g. `test/support`) or otherwise scoping them to the test environment.
- In the README v2 warning block, "The Purpose of v2" is capitalized mid‑sentence and the sentence about `opts` now reads a bit awkwardly split across lines; consider reverting to lower‑case "purpose" and tightening the line break to read more naturally.
## Individual Comments
### Comment 1
<location path="README.md" line_range="92-94" />
<code_context>
> passing of options (including `conn`). The breaking change here
-> is that `conn` is now part of `opts`. > when calling functions, see
-> example below:
+> is that `conn` is now part of `opts`. when calling functions,
+> see example below:
```elixir
</code_context>
<issue_to_address>
**suggestion (typo):** Fix sentence fragmentation and capitalization around "when calling functions".
The period before "when" turns the phrase into a fragment and leaves "when" uncapitalized. Either capitalize and keep it as one sentence ("When calling functions, see example below:") or merge it into the previous sentence ("is that `conn` is now part of `opts` when calling functions; see example below:").
```suggestion
> passing of options (including `conn`). The breaking change here
> is that `conn` is now part of `opts` when calling functions; see example below:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| > passing of options (including `conn`). The breaking change here | ||
| > is that `conn` is now part of `opts`. > when calling functions, see | ||
| > example below: | ||
| > is that `conn` is now part of `opts`. when calling functions, | ||
| > see example below: |
There was a problem hiding this comment.
suggestion (typo): Fix sentence fragmentation and capitalization around "when calling functions".
The period before "when" turns the phrase into a fragment and leaves "when" uncapitalized. Either capitalize and keep it as one sentence ("When calling functions, see example below:") or merge it into the previous sentence ("is that conn is now part of opts when calling functions; see example below:").
| > passing of options (including `conn`). The breaking change here | |
| > is that `conn` is now part of `opts`. > when calling functions, see | |
| > example below: | |
| > is that `conn` is now part of `opts`. when calling functions, | |
| > see example below: | |
| > passing of options (including `conn`). The breaking change here | |
| > is that `conn` is now part of `opts` when calling functions; see example below: |
Summary by Sourcery
Extract test schemas into dedicated modules, refine documentation, and tweak CI health‑check logging while noting Typesense v28.0 support in the changelog.
Enhancements:
CI:
Documentation: