Fix Rust SDK review issues#1492
Conversation
3ca1187 to
36906d2
Compare
Greptile SummaryThis PR makes two targeted fixes to the Rust SDK generator: it corrects Confidence Score: 4/5Safe to merge; only a minor style concern about the upper version bound on The nullable-param fix is correct and directly aligned with the Twig template's type generation logic. The dependency relaxation is a clear improvement over exact pins. The sole observation is a P2 style note about the No files require special attention beyond the P2 note on Important Files Changed
Reviews (1): Last reviewed commit: "Fix Rust SDK review issues" | Re-trigger Greptile |
| url = "=2.5.4" | ||
| idna = "=1.0.3" | ||
| idna_adapter = "=1.0.0" | ||
| url = ">=2.5.4, <2.6" |
There was a problem hiding this comment.
Upper bound may be unnecessarily restrictive for a library crate
url = ">=2.5.4, <2.6" (equivalent to ~2.5.4) pins the SDK to the 2.5.x patch series. If a downstream consumer's dependency graph resolves url to a 2.6.x release, it will conflict. For a library crate the idiomatic Rust approach is to use the widest range compatible with the stated MSRV — here that would be url = "2" or at minimum url = ">=2.5.4" — and let the top-level binary crate perform any tighter pinning via Cargo.lock. The rust-version = "1.83" field in the same file already communicates the compiler floor; no upper dependency bound is needed to enforce it unless a specific url 2.6 release is known to raise the MSRV above 1.83.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
What
Some(...)when the Rust method signature expectsOption<T>.url,idna, andidna_adapterdependencies from exact pins to bounded ranges that still respect Rust 1.83 compatibility.Why
Greptile flagged Rust SDK release review issues in appwrite/sdk-for-rust#13: optional policy examples did not compile, and exact dependency pins were too strict for a library crate.
The email-template PATCH route was checked against the current Appwrite route/spec and still correctly uses
/project/templates/emailwithtemplateIdin the body. The password-history typo is left untouched because it comes from the upstream spec.Testing
php example.php rustcomposer lint-twigvendor/bin/phpunit tests/Rust183Test.php