Skip to content

refactor(event): cleanup getter methods - #1310

Merged
Molter73 merged 1 commit into
mainfrom
mauro/refactor/cleanup-file-data-accesors
Jul 30, 2026
Merged

refactor(event): cleanup getter methods#1310
Molter73 merged 1 commit 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 file rename event handling to use a clearer structured format.
    • Improved consistency when accessing, comparing, and exporting rename event details.
    • Removed the separate rename data type in favor of directly representing new and previous file information.

@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

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Enterprise

Run ID: c1897c46-c73b-4805-b60b-2f96a52547d4

📥 Commits

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

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

📝 Walkthrough

Walkthrough

FileData::Rename now stores new and old BaseFileData fields directly. Event accessors, construction, serialization, equality, and monitoring logic were updated, and RenameFileData was removed.

Changes

Rename FileData representation

Layer / File(s) Summary
Rename variant construction and public shape
fact/src/event/mod.rs
The public rename variant and its construction and event-type matching use direct new and old fields.
Rename event accessors and state updates
fact/src/event/mod.rs
Rename detection, inode and path accessors, host-path mutation, and monitored-state access use the new struct fields.
Rename serialization and cleanup
fact/src/event/mod.rs
Protobuf, OpenTelemetry, and equality handling use both rename fields, while RenameFileData and its implementations are removed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: joukovirtanen, erthalion

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main refactor to FileData getter cleanup.
Description check ✅ Passed The PR description matches the template and includes a summary, checklist, and testing notes.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mauro/refactor/cleanup-file-data-accesors

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 (6ba96be) to head (ca94fc3).

Files with missing lines Patch % Lines
fact/src/event/mod.rs 0.00% 83 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #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 2 times, most recently 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
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 631caae to ca94fc3 Compare July 30, 2026 08:22
@Molter73
Molter73 disabled auto-merge July 30, 2026 13:21
@Molter73
Molter73 disabled auto-merge July 30, 2026 13:22
@Molter73
Molter73 merged commit ec9af6d into main Jul 30, 2026
42 checks passed
@Molter73
Molter73 deleted the mauro/refactor/cleanup-file-data-accesors branch July 30, 2026 13:23
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