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

Kernel logging #264

Merged
3 commits merged into from Feb 14, 2011
Merged

Kernel logging #264

3 commits merged into from Feb 14, 2011

Conversation

omazapa
Copy link
Contributor

@omazapa omazapa commented Feb 8, 2011

Implented logging in kernel to remove noise. It is needed to write the frontend in terminal without extra outputs.

@fperez
Copy link
Member

fperez commented Feb 11, 2011

Thanks Omar, this is looking much better. Can you make the other fix that Robert suggested? It' just a matter of moving the _logger from being a class attribute to a simple module-level global. Since logging is used by all objects in the module, there's no benefit in making it a private attribute of each object, it can just be a module-level global (and it can be named simply 'logger', without the underscore).

We're getting close to ready for merging, thanks for the good work!

@omazapa
Copy link
Contributor Author

omazapa commented Feb 11, 2011

ok guys is done, I am ready to continue with the frontend. Thanks!!

@fperez
Copy link
Member

fperez commented Feb 11, 2011

This looks good to me. Robert, do you have any other comments on this one? You have much more experience than I do with proper patterns of using the logging model, so before approving the request for merge, a final check from you would be much appreciated.

@rkern
Copy link
Contributor

rkern commented Feb 14, 2011

Looks good to me.

@fperez
Copy link
Member

fperez commented Feb 14, 2011

Thanks, Robert, for the check.

With that, I'll go ahead and merge it now, Omar. Good work!

@fperez
Copy link
Member

fperez commented Feb 14, 2011

Merged! Note, Omar, that the actual merge (9d65bd5) had to resolve a few conflicts because of changes in the session api since you started working on this. But I figured we'd made you work enough on this one already, so I finished that work :)

So go ahead and continue using master, which has the final form of the merge with your earlier commits and the last one fixed up by me to resolve the conflicts.

This pull request was closed.
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.

None yet

3 participants