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

Introduce abstraction for stream logging #70

Merged
merged 1 commit into from
Aug 22, 2017
Merged

Introduce abstraction for stream logging #70

merged 1 commit into from
Aug 22, 2017

Conversation

dbussink
Copy link
Contributor

This introduces a wrapper class to put the logic for logging to an IO like object. This is to prevent having to put all the logic in the log.rb class itself based if it's a syslog instance or not.

It updates the tests to account for this and also removes the now unused mutex code.

Fixes the broken syslog logging currently on master, which fails with the following error:

private method `print' called for #<Scrolls::SyslogLogger:0x007f50995aa578>

Opted for this approach over implementing print in the syslog class, since that would mean having to chop off the added newline there since syslog already handles the newline logic. By introducing the new class, this can be moved there to where the newline logic is needed.

This introduces a wrapper class to put the logic for logging to an IO
like object. This is to prevent having to put all the logic in the
log.rb class itself based if it's a syslog instance or not.

It updates the tests to account for this and also removes the now unused
mutex code.
@@ -130,7 +130,7 @@ def test_log_exception

oneline_backtrace = @out.string.gsub("\n", 'XX')

assert_match /test=exception at=exception.*test_log_exception.*XX.*minitest/,
assert_match /test=exception at=exception.*test_log_exception.*XX/,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was needed to get the tests to pass since more recent Ruby versions don't run the tests with minitest by default anymore.

@@ -231,21 +232,14 @@ def calc_time(start, finish)
((finish - start).to_f * @t)
end

def mtx
@mtx ||= Mutex.new
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mutex wasn't used anymore so it could be cleaned up.

@asenchi
Copy link
Owner

asenchi commented Aug 22, 2017

This looks legit to me, thanks @dbussink. I'm going to cut a new release at 0.3.9 for this fix, and then begin work to clean things up after being away from the project for some time.

@asenchi asenchi merged commit 9406704 into asenchi:master Aug 22, 2017
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.

2 participants