-
Notifications
You must be signed in to change notification settings - Fork 404
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
Update logging #1072
Update logging #1072
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## staging #1072 +/- ##
===========================================
- Coverage 78.90% 74.66% -4.24%
===========================================
Files 57 64 +7
Lines 1536 2333 +797
Branches 266 431 +165
===========================================
+ Hits 1212 1742 +530
- Misses 189 366 +177
- Partials 135 225 +90
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
Looks good to me, the only doubt that I have is about using directly the logger just for the debug. Couldn't we create a specific event for debugging? Just for the purpose of always passing through the EventManager?
I'm still not completely sure about using the EventManager. I tried for instance using the events in the monitor tests for example with Lines 193 to 197 in 302adae
I don't have a full grasp of Events in node but felt like the modules themselves (monitor etc.) emitting events is more sensible. Still we don't have to decide on this now and just get the job done for now. |
View in Huly HI-550