-
Notifications
You must be signed in to change notification settings - Fork 4
Fix for #614 - incorrect contract template. #618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughTwo template files were updated to fix code generation issues. The flipper template now generates deployment calls using the module name instead of the HostRef variant, resolving trait bound errors in generated tests. The CEP18 template had a trailing newline adjusted. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (2)
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 |
Benchmark report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Request Changes
This PR fixes a critical compilation error in contract tests by correcting the deploy method call, ensuring generated code compiles successfully, alongside a minor style improvement.
🌟 Strengths
- Accurately resolves the reported issue #614 with a minimal and correct change.
⚡ Key Risks & Improvements (P1)
- templates/flipper.rs.template: Corrects the deploy method call to prevent build failures in generated contracts.
💡 Suggestions (P2)
- templates/cep18.rs.template: Removes trailing whitespace to improve code style consistency and reduce diff noise.
⚠️ **Unanchored Suggestions (Manual Review Recommended)**
The following suggestions could not be precisely anchored to a specific line in the diff. This can happen if the code is outside the changed lines, has been significantly refactored, or if the suggestion is a general observation. Please review them carefully in the context of the full file.
📁 File: templates/cep18.rs.template
The change removes trailing whitespace at the end of the file, improving code style consistency. While this doesn't affect functionality, maintaining consistent formatting across templates reduces noise in diffs and follows Rust style conventions.
Related Code:
mod tests {
use super::*;
use odra::host::{Deployer, NoArgs};
#[test]
fn it_works() {
let env = odra_test::env();
let init_args = #init_args_struct_name {
#(#init_args_members),*
};
assert!(#module_name::try_deploy(&env, init_args).is_ok());
}
}
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| use super::*; | ||
| use odra::host::{Deployer, NoArgs}; | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1 | Confidence: High
The change fixes a critical compilation error in generated contract tests by replacing #module_nameHostRef::deploy with #module_name::deploy. The original code attempted to call deploy on a type that doesn't implement the required OdraContract trait, causing immediate build failures in projects using the template. This directly addresses issue #614 where cargo odra test would fail with trait bound errors after generating new contracts.
Closes #614
Summary by CodeRabbit
Style
Tests