feat(crates): add soar-events for frontend-agnostic event reporting#156
feat(crates): add soar-events for frontend-agnostic event reporting#156
Conversation
📝 WalkthroughWalkthroughAdds a new workspace crate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Cargo.toml`:
- Line 56: The workspace lists the dependency "soar-events" as version "0.1.0"
but the crate's own Cargo.toml has version "0.0.0", causing Cargo resolution to
fail; open the crate manifest for the soar-events crate and either change its
version field to "0.1.0" or adjust the workspace dependency to "0.0.0" so both
versions match (update the version = value in the workspace entry or the version
= value in crates/soar-events/Cargo.toml accordingly).
In `@crates/soar-events/src/lib.rs`:
- Around line 444-448: OperationFailed variant in enum SoarEvent is missing the
pkg_id field while all other per-package events include both pkg_name and
pkg_id; update the OperationFailed variant to include pkg_id (e.g., pkg_id:
<type>) in its definition, then update all creation sites (for example where
collector.emit(SoarEvent::OperationFailed { ... }) is called) to populate
pkg_id, and adjust any pattern matches/consumers that destructure
OperationFailed to handle the new pkg_id field so consumers can correlate
failures to package identifiers.
🧹 Nitpick comments (2)
crates/soar-events/src/event.rs (2)
36-53: Consider adding context fields toDownloadRetryandDownloadAborted.
DownloadRetrydoesn't carry an attempt number or the error that triggered the retry, andDownloadAborteddoesn't carry the final error reason. A GUI or CLI consumer would likely want to display "Retry 3/5: connection timed out" or "Aborted: max retries exceeded (last error: …)".💡 Suggested additions
DownloadRetry { op_id: OperationId, pkg_name: String, pkg_id: String, + attempt: u32, + error: String, }, DownloadAborted { op_id: OperationId, pkg_name: String, pkg_id: String, + error: String, },
254-266:BuildStagevariant ordering:Sandboxinglisted last but logically happens first.
SandboxingprecedesRunningin the build lifecycle, but it's the last variant in the enum. This doesn't affect functionality but reading order would better match chronological order.Suggested reorder
pub enum BuildStage { + /// Activating sandbox for build. + Sandboxing, /// Running build command N of M. Running { command_index: usize, total_commands: usize, }, /// Build command completed. CommandComplete { command_index: usize }, - /// Activating sandbox for build. - Sandboxing, }
…tic event reporting (pkgforge#156) ⌚
Introduces the soar-events crate with a typed event system that decouples operation progress reporting from any specific frontend, enabling both CLI and GUI consumers.
Summary by CodeRabbit
New Features
Chores