-
Notifications
You must be signed in to change notification settings - Fork 3
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
src: add log API #135
src: add log API #135
Conversation
0057589
to
2fc3e37
Compare
function writeLog(msg, sev) { | ||
binding.writeLog(typeof msg === 'string' ? msg : JSONStringify(msg), sev); | ||
} |
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.
Should we check other types as well such as Buffer? I don't really know tbh
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.
That's a good idea, but I wasn't sure if we should match how console.log()
works on output.
Add a C++ API to collect logs written with the nsolid JS API. PR-UR: #135 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
2fc3e37
to
9c79696
Compare
template <typename G> | ||
void on_log_write_hook_proxy_(SharedEnvInst inst, | ||
LogWriteInfo info, | ||
void* data) { | ||
(*static_cast<G*>(data))(inst, std::move(info)); | ||
} |
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.
Does the compiler know how to move LogWriteInfo or do we have to implement the move constructor ourselves?
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.
LogWriteInfo
contains only standard types that it knows how to move. So we should be fine.
Add a C++ API to collect logs written with the nsolid JS API.
Log support for Winston and Pino will also be added.