Make ResourceStorage trait &self-based and remove 'static requirement#4188
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the resource storage system by changing ResourceStorage methods to take shared references (&self) instead of mutable references (&mut self), and updating ResourceFuture to use a lifetime parameter instead of being 'static. Consequently, Box<dyn ResourceStorage> is replaced with Arc<dyn ResourceStorage> across the codebase, simplifying resource handling and removing the need for RwLock wrappers. The feedback suggests a minor cleanup in desktop/src/app.rs to use the imported Arc::new directly instead of its fully qualified path.
There was a problem hiding this comment.
1 issue found across 12 files
Confidence score: 2/5
- There is a high-confidence, high-severity concern in
node-graph/graph-craft/src/application_io.rs: using.expect()can still panic whenresourcesisNone, which creates a clear runtime regression risk. - Because this is a concrete crash path (severity 8/10, confidence 9/10) in updated logic, the merge risk is elevated until error handling is made graceful instead of panicking.
- Pay close attention to
node-graph/graph-craft/src/application_io.rs- ensure theresources == Nonepath returns a handled error rather than triggering a panic.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
No description provided.