refactor(event): cleanup getter methods - #1310
Conversation
Adds an "/inodes" endpoint that returns the contents of the userspace inode map for inspection. This will be useful in tests that need to look into the internal state after operations that don't emit events or during development to validate what is currently being tracked. The new endpoint is disabled by default and can be enabled with the new `endpoint.introspection` configuration value, that should gate any future introspection endpoints we decide to add in the future. This configuration value is not hotreloadable, it can only be set at start up, this is meant to make it harder for the endpoint to come online on production environments.
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## mauro/feat/inodes-introspection #1310 +/- ##
===================================================================
+ Coverage 35.42% 35.48% +0.06%
===================================================================
Files 22 22
Lines 3241 3235 -6
Branches 3241 3235 -6
===================================================================
Hits 1148 1148
+ Misses 2088 2082 -6
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
df223d4 to
7089c0c
Compare
This is a small refactor done ahead of #1059, cleaning up getters in the `FileData` type by using better pattern matching. The `Rename` variant is also streamlined by using a struct variant instead of a tuple holding `RenameFileData`.
7089c0c to
631caae
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
fact/src/event/mod.rs (1)
165-192: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding unit tests given 0% patch coverage on this refactor.
Codecov flagged 0% patch coverage across the 83 changed lines here. The getters/setters rely on large or-patterns spanning 11 variants each; a future edit that misplaces
new/old(or drops a variant from one of the shared or-patterns) would silently compile but return wrong data (e.g.get_inodereturning the "old" inode instead of "new"). A few targeted unit tests aroundEvent::get_inode/get_old_inode/get_filename/get_old_filenamefor theRenamecase would catch that class of regression cheaply.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fact/src/event/mod.rs` around lines 165 - 192, Add targeted unit tests for the Rename variant covering Event::get_inode, Event::get_old_inode, Event::get_filename, and Event::get_old_filename. Construct a rename event with distinct old and new inode and filename values, then assert each getter returns the corresponding value, validating the shared or-pattern mappings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@fact/src/event/mod.rs`:
- Around line 165-192: Add targeted unit tests for the Rename variant covering
Event::get_inode, Event::get_old_inode, Event::get_filename, and
Event::get_old_filename. Construct a rename event with distinct old and new
inode and filename values, then assert each getter returns the corresponding
value, validating the shared or-pattern mappings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 4f74ba34-40ab-49a1-8449-4de8de79bf48
📒 Files selected for processing (1)
fact/src/event/mod.rs
Description
This is a small refactor done ahead of #1059, cleaning up getters in the
FileDatatype by using better pattern matching. TheRenamevariant is also streamlined by using a struct variant instead of a tuple holdingRenameFileData.Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
CI is enough.
Summary by CodeRabbit
newandoldfile details.