Skip to content

Conversation

tdeekens
Copy link
Member

Adds a scribe to the utils' logger which prints every log statement onto the screen. This scribe will be used in lmvc to replace all the library's print-statements.

Checklist

  • Add ScreenScribe class and implement it
  • Update config.json and sample-config.json
  • Test the scribe

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d7933aa on feature/logger-screenscribe into 05aa001 on master.

@ghost ghost assigned tdeekens Nov 18, 2013
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 6915fb3 on feature/logger-screenscribe into 05aa001 on master.

@tdeekens
Copy link
Member Author

@konstantinos-chronis ready for review.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to make the code more clear to read I propose the following

if($this->_omitMessage($level))
return false;

And then continue with the rest of the code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how currently every scribe homogeneously handles its local logging level. Matter of taste I guess. I'll ask my inner coding-zen later.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.35%) when pulling 344abd0 on feature/logger-screenscribe into 05aa001 on master.

@tdeekens
Copy link
Member Author

@konstantinos-chronis smaller(..) is added and called by AbstractScribe. I don't like this is[A-z] thing for functions returning booleans. It's in the docs anyways and the IDE will tell you.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smaller should be used here too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. 👍 Happy now?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.35%) when pulling 27b960d on feature/logger-screenscribe into 05aa001 on master.

@tdeekens tdeekens merged commit 27b960d into master Nov 20, 2013
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