feat: genericize test macro#133
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds unified testing proc-macros ( Changes
Sequence Diagram(s)sequenceDiagram
participant Macro as "Proc-Macro (doctest/test)"
participant Parser as "Test Parser"
participant Codegen as "Test Codegen"
participant Migrator as "Migrator"
participant DB as "Database Pool"
participant Test as "User Test"
participant Cleanup as "Cleanup"
Macro->>Parser: receive async fn (test/doctest)
Parser->>Parser: analyze fn name, param, body, backend(s)
Parser->>Codegen: produce TestAnalysis
Codegen->>DB: resolve migration path & create pool (sqlite/postgres/mysql)
Codegen->>Migrator: apply migrations to pool
Codegen->>Test: invoke user test body with initialized pool
Test->>Cleanup: test ends, trigger cleanup
Cleanup->>DB: drop temp DB (postgres/mysql) or noop (sqlite)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #133 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 29 30 +1
Lines 2929 3418 +489
==========================================
+ Hits 2929 3418 +489 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fabrique-core/src/__private.rs`:
- Around line 126-132: replace_db_in_url currently drops query/fragment parts
because it rsplit_once on '/' across the whole URL; instead, first separate the
query/fragment component (everything starting at '?' or '#') from the main URL
string, perform the rsplit_once('/') replacement only on the main part to swap
the last path segment (the DB name), then reattach the preserved query/fragment.
Update the function replace_db_in_url to locate the first '?' or '#' (if
present), split into main and suffix, do the existing rsplit_once logic on main
to produce "{base}/{db_name}", and finally return "{new_main}{suffix}" so
connection params like ?sslmode are retained.
In `@fabrique-derive/src/test.rs`:
- Around line 298-317: Replace the fragile substring check in workspace_root():
instead of using content.contains("[workspace]"), parse the Cargo.toml content
with a TOML parser (e.g., toml::from_str into toml::Value) and check for the
presence of the "workspace" table (value.get("workspace").is_some()) so the
detection is robust against occurrences in strings/comments; keep the rest of
the loop and the panic behavior intact and reference the cargo_toml and content
variables and the workspace_root() function when making the change.
- Around line 182-196: The generated test currently appends the cleanup block
after the body in generate(), so a panic in the test body skips cleanup; fix by
emitting a Drop-based guard struct in the generated test (e.g. TestCleanup) and
instantiate it before emitting #(`#stmts`)* so its Drop runs during unwind; update
generate() to include the guard type and an instance created right after
`#pool_setup` and remove the direct trailing `#cleanup` placement—use
generate_cleanup() to produce the cleanup code body used by the
TestCleanup::drop impl so cleanup always runs even on panic.
- Line 37: Rename the fallible constructor TestAnalysis::from to a clearer name
(e.g., parse or try_from_item) to avoid confusion with the standard From trait:
change the pub fn from(&ItemFn) -> Result<Self, syn::Error> declaration to pub
fn parse(...) or pub fn try_from_item(...), update all call sites that invoke
TestAnalysis::from(...) to the new name, and update any documentation, tests, or
re-exports referencing that symbol so the signature and behavior remain the same
but the name no longer shadows the trait method.
In `@fabrique/target/.rustc_info.json`:
- Line 1: The committed build artifact fabrique/target/.rustc_info.json should
be removed from version control and the .gitignore updated so nested target/
directories are ignored; remove the tracked file (e.g., git rm --cached
fabrique/target/.rustc_info.json or equivalent), commit that removal, and change
the existing ignore entry '/target' in .gitignore to 'target/' so any target/
directory at any level (including fabrique/target/) is ignored going forward.
In `@fabrique/tests/model.rs`:
- Line 14: Rename the test parameter from connection to pool to match the rest
of the test suite: update the async fn test_save signature to use pool:
Pool<Sqlite> and replace all usages of connection inside test_save (and any
other affected tests in this file, e.g., the other test function declared
similarly) with pool so names are consistent across the PR.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fabrique-core/src/__private.rs`:
- Around line 117-124: temp_db_name currently builds a name from SystemTime
nanos + std::process::id(), which can collide for concurrent calls in the same
process; replace the time-based uniqueness with a process-wide thread-safe
unique generator (e.g., a static AtomicU64 counter incremented with fetch_add or
a UUID) to guarantee uniqueness across threads and async tests. Modify the
temp_db_name function to use a static AtomicU64 (or call a UUID generator) to
produce the per-call suffix instead of relying solely on SystemTime, keeping the
"fabrique_test_{pid}_{suffix}" format and referencing the temp_db_name function
for the change.
In `@fabrique/tests/query_builder.rs`:
- Line 886: Tests that exercise SQL RETURNING currently use Pool<Sqlite> only,
skipping Postgres; update the test attributes so RETURNING is exercised on
Postgres as well while still excluding MySQL. Replace occurrences of
Pool<Sqlite> test gating (and the comment) by using the test macro attribute to
include postgres (e.g., #[fabrique::test(backends(sqlite, postgres))] or
#[cfg(not(feature = "mysql"))]) for the tests that reference RETURNING, and
ensure the test signatures that currently mention Pool<Sqlite> accept
Pool<Postgres> too so the RETURNING path runs on both sqlite and postgres
(preserving exclusion of mysql).
---
Duplicate comments:
In `@fabrique-core/src/__private.rs`:
- Around line 126-132: The function replace_db_in_url drops query parameters
because rsplit_once('/') operates over the whole URL; fix replace_db_in_url to
preserve any query string (and fragment) by first splitting the input url into
main_part and optional suffix (split at the first '?' or '#' as needed), perform
the rsplit_once('/') replacement on the main_part to swap the DB segment, then
reattach the suffix (e.g., "?..." or "#...") so the resulting String contains
the new database name plus the original query/fragment; update the logic in
replace_db_in_url accordingly.
In `@fabrique-derive/src/test.rs`:
- Around line 37-78: Rename the fallible constructor TestAnalysis::from to a
clearer name (e.g., TestAnalysis::parse or TestAnalysis::try_from_item) to avoid
confusion with the From trait; update the impl block header and all call sites
that invoke TestAnalysis::from to use the new name, keeping the signature and
behavior identical (still returns Result<Self, syn::Error>) and ensure
references to related helpers like TestAnalysis::parse_backend remain valid.
- Around line 388-407: The workspace_root() function currently detects a
workspace by string-matching "[workspace]" which is fragile; instead parse each
Cargo.toml with a TOML parser and check for a workspace table. Update
workspace_root() to read the file into a string, parse it with a TOML parser
(e.g., toml::from_str or cargo_toml::Manifest), and test for the presence of a
top-level "workspace" table/key before returning the path; ensure error handling
wraps parse failures with a clear message and only treat a valid workspace table
as a match.
- Around line 322-337: generate_single and generate_multi append the cleanup
Tokens after the test body so a panic during the test (assertions or unwrap)
skips cleanup and leaves orphaned databases; wrap the test body so cleanup
always runs — either create an RAII guard (e.g., a local struct Drop guard
produced by pool_setup_tokens() or a new guard variable) that calls
cleanup_tokens() in its Drop, or execute the test body inside
std::panic::catch_unwind and call the cleanup in a finally-like path before
re-propagating the panic; update pool_setup_tokens()/generate_* to return/inject
the guard (or wrap the quoted body with catch_unwind and then call
cleanup_tokens(`#cleanup`) in both success and error branches) so cleanup is
guaranteed even on panic (referencing generate_single, generate_multi,
pool_setup_tokens, cleanup_tokens, analysis.body.stmts, param, migration_path).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fabrique-core/src/__private.rs`:
- Around line 33-57: create_postgres_pool currently creates the temp DB (using
temp_db_name and CREATE DATABASE via sqlx::raw_sql) and then runs migrations
(Migrator::new(...).run(...)), but if the migration step fails the newly created
DB is leaked; modify create_postgres_pool so that after successful CREATE
DATABASE you ensure any error from Migrator::new or .run triggers cleanup:
connect to the base URL (or reuse base_pool if retained) and execute a DROP
DATABASE "{db_name}" before returning the error, propagating the original
migration error (optionally wrapping any drop error or logging it); apply the
same pattern to create_mysql_pool (use the equivalent connection and DROP
DATABASE SQL for MySQL) so temporary DBs are removed on migration failure.
- Around line 73-115: The generated tests must explicitly drop DB connection
pools returned by create_mysql_pool before calling cleanup_test_db_mysql to
avoid DROP DATABASE failing due to open connections: after using the pool
returned by create_mysql_pool call drop(pool) (or drop(pg_pool) for Postgres)
before invoking cleanup_test_db_mysql(&base_url, &db_name) (and
cleanup_test_db_postgres when present) so the connection is closed prior to the
DROP; update test generation to insert this drop() call immediately before
cleanup.
---
Duplicate comments:
In `@fabrique-core/src/__private.rs`:
- Around line 117-124: temp_db_name currently builds a temporary name from
process::id() and SystemTime nanos which can collide for concurrent async tests;
introduce a static atomic counter (e.g., a once-initialized static AtomicU64)
and include a fetch_add() value in the formatted name inside temp_db_name to
ensure uniqueness across concurrent calls (keep using std::process::id() and
timestamp but append the atomic counter to eliminate nanosecond collision).
- Around line 126-134: The function replace_db_in_url currently drops URL query
parameters; update replace_db_in_url to split off the query (and fragment if
present) from the input URL before locating and replacing the last path segment
so you only swap the database segment and then reattach the original
query/fragment; locate the logic in replace_db_in_url (uses find("://") and
rfind('/')) and modify it to parse/slice at the first '?' or '#' after the host
portion, perform the replacement on the path-only portion, then append the saved
query/fragment back; also add the suggested unit test
replace_db_in_url_preserves_query_params to verify queries are preserved.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
fabrique-derive/src/test.rs (1)
329-344: Postgres/MySQL cleanup is skipped on test panic — orphaned databases will accumulate.This was flagged in a prior review round but appears unaddressed. In both
generate_singleandgenerate_multi, the cleanup block follows the user's test statements. Any panic (assertion failure,.unwrap(), etc.) will unwind past the cleanup, leaving orphaned databases in CI.A drop guard or
catch_unwindwrapper would ensure cleanup always runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fabrique-derive/src/test.rs` around lines 329 - 344, The generated tests (generate_single and generate_multi) place `#cleanup` after the user's test statements so any panic/unwind skips cleanup; modify these functions to ensure cleanup always runs by wrapping the test body in a panic-safe guard (e.g., create a drop guard or use std::panic::catch_unwind around the `#(`#stmts`)*` execution) so that `cleanup_tokens(backend)` executes in a finally-like path; locate generate_single/generate_multi and replace the current sequence of `#setup`, `#(`#stmts`)*`, `#cleanup` with an arrangement that runs `#cleanup` from a Drop guard or within a catch_unwind unwind-safe block, referencing pool_setup_tokens and cleanup_tokens when constructing the guard/closure.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
fabrique-derive/src/test.rs
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fabrique-derive/src/test.rs`:
- Around line 295-326: generate_doctest currently only rejects Backend::Multi
but always emits a call to ::fabrique::__private::create_sqlite_pool while using
self.migration_path; update generate_doctest to reject any non-Sqlite concrete
backend instead: check self.analysis.backend and if it is not Backend::Sqlite
return a syn::Error::new_spanned(&self.analysis.fn_name, "#[fabrique::doctest]
only supports Sqlite concrete backends") .into_compile_error(); this keeps the
existing create_sqlite_pool emission for Sqlite and prevents generating code
that applies Postgres migrations to an in-memory SQLite pool.
- Around line 634-659: The test test_generate_multi_backend currently calls
TestCodegen::new (which uses workspace_root() and resolve_migration_path) making
it non-hermetic; change it to use TestCodegen::with_path and pass a controlled
fake workspace/migration path so the test does not depend on real filesystem
layout, and update assertions to explicitly check the generated migration path
segments (or add a small separate test for resolve_migration_path) to assert
backend-specific paths for Sqlite/Postgres/MySql; reference TestCodegen::new,
TestCodegen::with_path, workspace_root(), and resolve_migration_path to find the
code to change.
---
Duplicate comments:
In `@fabrique-derive/src/test.rs`:
- Around line 329-344: The generated tests (generate_single and generate_multi)
place `#cleanup` after the user's test statements so any panic/unwind skips
cleanup; modify these functions to ensure cleanup always runs by wrapping the
test body in a panic-safe guard (e.g., create a drop guard or use
std::panic::catch_unwind around the `#(`#stmts`)*` execution) so that
`cleanup_tokens(backend)` executes in a finally-like path; locate
generate_single/generate_multi and replace the current sequence of `#setup`,
`#(`#stmts`)*`, `#cleanup` with an arrangement that runs `#cleanup` from a Drop
guard or within a catch_unwind unwind-safe block, referencing pool_setup_tokens
and cleanup_tokens when constructing the guard/closure.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
fabrique-derive/src/test.rs (5)
638-663:test_generate_multi_backenddepends on real filesystem viaTestCodegen::new.Already noted in a prior review — unlike other codegen tests that use
TestCodegen::with_path, this one hits the real workspace layout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fabrique-derive/src/test.rs` around lines 638 - 663, The test_generate_multi_backend unit test uses TestCodegen::new which touches the real workspace layout; change it to use the file-system-isolated helper (TestCodegen::with_path or the existing temp-dir based constructor) so the test no longer depends on the real workspace. Specifically, in the test_generate_multi_backend body replace the TestCodegen::new(&analysis) call with TestCodegen::with_path(&analysis, temp_dir_path) (or the repo's temp helper) so the codegen runs against a temporary test fixture; ensure any required test fixture files are created under that temp path before calling generate() and clean up afterwards.
50-50:TestAnalysis::fromnaming shadowsFromtrait convention.Already noted in a prior review —
fromon a type that returnsResultreads like the infallibleFromtrait. Considerparseortry_from_item.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fabrique-derive/src/test.rs` at line 50, The method TestAnalysis::from returns Result and therefore should not use the infallible From-style name; rename the function (e.g., TestAnalysis::parse or TestAnalysis::try_from_item) and update all call sites and tests to use the new name while preserving the signature Result<Self, syn::Error>; also update any doc/comments and imports referencing TestAnalysis::from to the new identifier to avoid shadowing the standard From convention.
395-418:content.contains("[workspace]")heuristic for workspace detection.Already noted in a prior review — this could theoretically match inside a string value or comment, though unlikely in practice for
Cargo.toml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fabrique-derive/src/test.rs` around lines 395 - 418, The current find_workspace_root uses a brittle text search (content.contains("[workspace]")) which can false-positive inside strings/comments; change it to properly parse Cargo.toml with a TOML parser (e.g., the toml crate) and check for a top-level "workspace" table/entry instead. Update find_workspace_root (and any callers like workspace_root) to read the file, parse with toml::from_str or toml::Value::from_str, and test that the parsed Value contains a "workspace" key at the top level before returning the path; fall back to the existing loop/panic behavior on parse errors or absence of the key.
295-326:generate_doctesthardcodescreate_sqlite_poolfor all concrete backends.Already noted in a prior review — if the user's function takes
Pool<Postgres>, this will emit SQLite pool creation with the Postgres migration path, failing at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fabrique-derive/src/test.rs` around lines 295 - 326, generate_doctest currently always emits a call to ::fabrique::__private::create_sqlite_pool which breaks when the user's function expects a non-SQLite Pool (e.g., Pool<Postgres>); update generate_doctest to inspect self.analysis.backend (or the concrete backend enum variant) and emit the correct pool-creation call for each concrete backend (e.g., create_postgres_pool/create_mysql_pool/create_sqlite_pool) or delegate to a helper like emit_create_pool_for_backend(backend, migration_path) so the generated doctest uses the matching pool creation expression for the detected backend and still returns the same TokenStream shape with the created pool bound to param and using migration_path.
329-344: Cleanup is skipped on test panic for Postgres/MySQL.Already noted in a prior review — if the test body panics, the cleanup tokens on line 341 are never reached, leaving orphaned test databases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fabrique-derive/src/test.rs` around lines 329 - 344, The generated test currently places cleanup code after the test body so a panic in the body skips cleanup; change generation in generate_single so cleanup always runs: instead of appending cleanup_tokens(...) after #(`#stmts`)*, create a RAII guard or use tokio::spawn + drop semantics produced by cleanup_tokens (e.g., have cleanup_tokens generate a let _cleanup_guard = CleanupGuard::new(param, backend, path); with a Drop impl that performs the teardown) and insert that binding before the test body (so it is dropped on panic), or alternatively wrap the body in std::panic::AssertUnwindSafe(...).catch_unwind().await and call cleanup in a finally branch; update generate_single to insert that guard/binding before #(`#stmts`)* (and adjust cleanup_tokens/CleanupGuard accordingly) so cleanup runs even if the test panics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fabrique-derive/src/test.rs`:
- Around line 821-827: The test
test_find_workspace_root_panics_without_workspace creates a temp dir and never
removes it; update the test to clean up the directory after the assertion by
either using tempfile::TempDir for automatic removal or by registering a scope
guard (e.g., scopeguard::defer) / calling std::fs::remove_dir_all at the end of
the test so the directory created for the test (fabrique_test_no_workspace) is
removed; keep the call to find_workspace_root(dir) and ensure cleanup runs even
if the test panics.
- Around line 422-428: The function expand_test currently ignores the _attr
TokenStream; instead validate and reject unexpected attributes by checking if
!_attr.is_empty() and returning a syn::Error. Concretely, in expand_test (keep
the same param name or rename _attr to attr) add: if !attr.is_empty() { return
Err(syn::Error::new_spanned(attr, "unexpected attributes for
#[fabrique::test]")); } before parsing the item so the macro emits a compile
error when users pass any arguments.
- Around line 349-375: generate_multi is repeatedly calling
resolve_migration_path (which calls workspace_root()) inside the
CONCRETE_BACKENDS iterator, causing redundant workspace traversal; hoist the
workspace root lookup and/or precompute migration paths before mapping over
backends (e.g. call workspace_root() once and pass it into
resolve_migration_path or build a Vec of (backend, path) entries), then use
those precomputed paths inside the iterator so resolve_migration_path/
workspace_root() is not invoked per backend.
---
Duplicate comments:
In `@fabrique-derive/src/test.rs`:
- Around line 638-663: The test_generate_multi_backend unit test uses
TestCodegen::new which touches the real workspace layout; change it to use the
file-system-isolated helper (TestCodegen::with_path or the existing temp-dir
based constructor) so the test no longer depends on the real workspace.
Specifically, in the test_generate_multi_backend body replace the
TestCodegen::new(&analysis) call with TestCodegen::with_path(&analysis,
temp_dir_path) (or the repo's temp helper) so the codegen runs against a
temporary test fixture; ensure any required test fixture files are created under
that temp path before calling generate() and clean up afterwards.
- Line 50: The method TestAnalysis::from returns Result and therefore should not
use the infallible From-style name; rename the function (e.g.,
TestAnalysis::parse or TestAnalysis::try_from_item) and update all call sites
and tests to use the new name while preserving the signature Result<Self,
syn::Error>; also update any doc/comments and imports referencing
TestAnalysis::from to the new identifier to avoid shadowing the standard From
convention.
- Around line 395-418: The current find_workspace_root uses a brittle text
search (content.contains("[workspace]")) which can false-positive inside
strings/comments; change it to properly parse Cargo.toml with a TOML parser
(e.g., the toml crate) and check for a top-level "workspace" table/entry
instead. Update find_workspace_root (and any callers like workspace_root) to
read the file, parse with toml::from_str or toml::Value::from_str, and test that
the parsed Value contains a "workspace" key at the top level before returning
the path; fall back to the existing loop/panic behavior on parse errors or
absence of the key.
- Around line 295-326: generate_doctest currently always emits a call to
::fabrique::__private::create_sqlite_pool which breaks when the user's function
expects a non-SQLite Pool (e.g., Pool<Postgres>); update generate_doctest to
inspect self.analysis.backend (or the concrete backend enum variant) and emit
the correct pool-creation call for each concrete backend (e.g.,
create_postgres_pool/create_mysql_pool/create_sqlite_pool) or delegate to a
helper like emit_create_pool_for_backend(backend, migration_path) so the
generated doctest uses the matching pool creation expression for the detected
backend and still returns the same TokenStream shape with the created pool bound
to param and using migration_path.
- Around line 329-344: The generated test currently places cleanup code after
the test body so a panic in the body skips cleanup; change generation in
generate_single so cleanup always runs: instead of appending cleanup_tokens(...)
after #(`#stmts`)*, create a RAII guard or use tokio::spawn + drop semantics
produced by cleanup_tokens (e.g., have cleanup_tokens generate a let
_cleanup_guard = CleanupGuard::new(param, backend, path); with a Drop impl that
performs the teardown) and insert that binding before the test body (so it is
dropped on panic), or alternatively wrap the body in
std::panic::AssertUnwindSafe(...).catch_unwind().await and call cleanup in a
finally branch; update generate_single to insert that guard/binding before
#(`#stmts`)* (and adjust cleanup_tokens/CleanupGuard accordingly) so cleanup runs
even if the test panics.
robinstraub
changed the title
Merged
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
1 participant
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.
resolves #132
Summary by CodeRabbit
New Features
Documentation
Chores