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

Use Logs library for logging #199

Merged
merged 1 commit into from May 3, 2016

Conversation

Projects
None yet
3 participants
@talex5
Copy link
Contributor

talex5 commented May 3, 2016

Note: to avoid breaking the API, this patch does not yet remove the (now-unused) console arguments from the signatures. This is so that this patch can be merged quickly to avoid creating more conflicts with other refactoring work going on.

Use Logs library for logging
Note: to avoid breaking the API, this patch does not yet remove the
console arguments from the public API.

@talex5 talex5 referenced this pull request May 3, 2016

Closed

[Tracking] Remove console #200

@yomimono yomimono merged commit a54f638 into mirage:master May 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@yomimono

This comment has been minimized.

Copy link
Member

yomimono commented May 3, 2016

Looks great to me! Thanks!

@hannesm

This comment has been minimized.

Copy link
Member

hannesm commented May 4, 2016

this is great! I'm uncertain about choice of log levels (should we come up with some guidelines? is info a reasonable default for all the messages?). and there's still a custom logging framework in tcp https://github.com/mirage/mirage-tcpip/blob/master/tcp/log.ml -- which should now use logs, shouldn't it?

@talex5

This comment has been minimized.

Copy link
Contributor

talex5 commented May 4, 2016

I kept most of them at info to avoid changing the behaviour too much in this PR, but feel free to make additional PRs to discuss adjustments to them.

The pre-existing logging does some stats too, so might need a bit of adjusting to move it over (but would be welcome I think). @samoht knows more about this...

samoht pushed a commit to samoht/mirage-tcpip that referenced this pull request Apr 4, 2017

Merge pull request mirage#199 from talex5/logs
Use Logs library for logging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment