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

Adding logging & metric recording configuration via settings #546

Closed
wants to merge 4 commits into from

Conversation

hbeeken
Copy link
Contributor

@hbeeken hbeeken commented Jan 29, 2015

This is where I've got to so far. I believe it's ready to go.

Note part of this pull request is to change the log method level from "log" to "info" as this fits with the other levels. Ideally we'd change the node.log method to be node.info but that's a very far reaching change. This change of level name has required some changes to the tests, however, by inspection doesn't cause any failures in the web-nodes tests.

Also, the formatting of the output isn't right but that's a different discussion :-)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.19%) to 89.34% when pulling d1eb7bd on hbeeken:log-settings into f983e4d on node-red:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 89.49% when pulling 7e83bb54bbceda16b307733b590799e27f31dea3 on hbeeken:log-settings into 5e5a220 on node-red:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 89.49% when pulling ba0c49c5aa3e57a3d71952a857e527077ffe2def on hbeeken:log-settings into 5e5a220 on node-red:master.

@hbeeken
Copy link
Contributor Author

hbeeken commented Feb 2, 2015

I've made a couple of changes to this pull request. I haven't squashed the commits because it got messy after I merged in the latest changes from HEAD. I thought it cleaner to spell out what each change does. Should be good to go now.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 89.57% when pulling d7e5498 on hbeeken:log-settings into 5e5a220 on node-red:master.

@hbeeken
Copy link
Contributor Author

hbeeken commented Feb 2, 2015

@knolleary I know you've been making some changes to this initial pull request locally, hopefully the bits and pieces I've done today hasn't messed that up too much. I've mainly been adding tests for the log level logic.

@knolleary
Copy link
Member

I pulled these changes to my local repository, made some additions and then pushed to master last night. I had expected that to close this PR as your commits were now included in master... but I realise the technique I used to grab your changes caused them to get a different commit hash, so github doesn't know I've already applied these changes.

Long story short.... these changes are in, albeit not via this exact PR.

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.

3 participants