Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Prepare to have more than one event type #77

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Conversation

eseidel
Copy link
Contributor

@eseidel eseidel commented Sep 7, 2023

Refactored our PatchInstallEvent to be just PatchEvent
and allow us to have more than one event type.

I moved it into its own events.rs file and added privacy
warnings to both this and the network code (eventually
we may move all this into a separate privacy.rs file).

I also fixed our build.rs file to not panic when our build
is otherwise broken. For whatever reason that seems to
cause the rust-analyzer to stop and not show any future
warnings (thus it's hard to fix things).

I moved away from having a "new" method for PatchEvent
with a series of Strings (which could be confused in order)
and instead am now using just the normal default struct
construction which effectively has named args.

Since EventType is now an enum, it's easy to use the
right enum not have to call PatchInstallEvent::new to
get the right magic string for "identifier".

Also added a bit more documentation to c_api.rs

Description

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Refactored our PatchInstallEvent to be just PatchEvent
and allow us to have more than one event type.

I moved it into its own events.rs file and added privacy
warnings to both this and the network code (eventually
we may move all this into a separate privacy.rs file).

I also fixed our build.rs file to not panic when our build
is otherwise broken. For whatever reason that seems to
cause the rust-analyzer to stop and not show any future
warnings (thus it's hard to fix things).

I moved away from having a "new" method for PatchEvent
with a series of Strings (which could be confused in order)
and instead am now using just the normal default struct
construction which effectively has named args.

Since EventType is now an enum, it's easy to use the
right enum not have to call PatchInstallEvent::new to
get the right magic string for "identifier".

Also added a bit more documentation to c_api.rs
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Patch coverage: 98.00% and project coverage change: -0.07% ⚠️

Comparison is base (292f978) 97.53% compared to head (f85ff52) 97.47%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #77      +/-   ##
==========================================
- Coverage   97.53%   97.47%   -0.07%     
==========================================
  Files           9       10       +1     
  Lines        1864     1858       -6     
==========================================
- Hits         1818     1811       -7     
- Misses         46       47       +1     
Files Changed Coverage Δ
library/src/c_api.rs 99.49% <ø> (ø)
library/src/events.rs 90.90% <90.90%> (ø)
library/src/network.rs 98.96% <100.00%> (-0.06%) ⬇️

☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.

@eseidel eseidel merged commit 9d4ce55 into main Sep 7, 2023
6 of 7 checks passed
@eseidel eseidel deleted the events branch September 7, 2023 19:31
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.

2 participants