Skip to content

refactor(event): cleanup getter methods - #1310

Open
Molter73 wants to merge 3 commits into
mainfrom
mauro/refactor/cleanup-file-data-accesors
Open

refactor(event): cleanup getter methods#1310
Molter73 wants to merge 3 commits into
mainfrom
mauro/refactor/cleanup-file-data-accesors

Conversation

@Molter73

@Molter73 Molter73 commented Jul 29, 2026

Copy link
Copy Markdown
Member

Description

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.

Checklist

  • Patch has a change log entry OR does not need one.
  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

CI is enough.

Summary by CodeRabbit

  • Refactor
    • Updated rename event data handling to use separate new and old file details.
    • Improved consistency when reading, updating, comparing, and exporting rename events.
    • Removed the previously exposed rename-specific wrapper type.

Molter73 added 2 commits July 27, 2026 15:14
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.
@Molter73
Molter73 requested a review from a team as a code owner July 29, 2026 09:09
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately reflects the getter refactor in FileData.
Description check ✅ Passed The description matches the template and includes the change summary, checklist, and testing notes.

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

codecov-commenter commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.48%. Comparing base (91987cb) to head (631caae).

Files with missing lines Patch % Lines
fact/src/event/mod.rs 0.00% 83 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Molter73
Molter73 force-pushed the mauro/refactor/cleanup-file-data-accesors branch from df223d4 to 7089c0c Compare July 29, 2026 09:15
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`.
@Molter73
Molter73 force-pushed the mauro/refactor/cleanup-file-data-accesors branch from 7089c0c to 631caae Compare July 29, 2026 09:23

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
fact/src/event/mod.rs (1)

165-192: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider 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_inode returning the "old" inode instead of "new"). A few targeted unit tests around Event::get_inode/get_old_inode/get_filename/get_old_filename for the Rename case 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

📥 Commits

Reviewing files that changed from the base of the PR and between 91987cb and 631caae.

📒 Files selected for processing (1)
  • fact/src/event/mod.rs

Base automatically changed from mauro/feat/inodes-introspection to main July 29, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants