-
Notifications
You must be signed in to change notification settings - Fork 50
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
Switch from arrays to DTO classes when passing event information around #94
Conversation
src/Provider/Doctrine/Auditing/Transaction/TransactionHydrator.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janklan Thanks for this nice contribution!
src/Provider/Doctrine/Auditing/Transaction/TransactionHydrator.php
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #94 +/- ##
==========================================
- Coverage 95.75% 94.47% -1.29%
==========================================
Files 35 40 +5
Lines 1390 1429 +39
==========================================
+ Hits 1331 1350 +19
- Misses 59 79 +20
Continue to review full report at Codecov.
|
I'm not sure why would the CI fail with exit code 8... :/ |
Followup to #91 (comment), here is the PR that uses DTO classes instead of arrays in order to improve code robustness.
I added a dedicated method to each event type. It expands the
Transaction
API surface a bit, but it also enables static code analysers to do their thing. Comparing to passing an array with vague structure that's later magically converted to something useful, I think the surface expansion is an acceptable cost to pay.