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

Do not fail in Loga::Event#to_s when there is no timestamp #88

Closed
wants to merge 1 commit into from

Conversation

bliof
Copy link
Contributor

@bliof bliof commented Nov 1, 2016

No description provided.

@bliof
Copy link
Contributor Author

bliof commented Nov 1, 2016

There is also some problem with timecop config when I run the specs locally:

  2) Loga::Event#to_s when no exception outputs the message
     Failure/Error: expect(subject.to_s).to eql("#{time_anchor.iso8601(3)} Hello World")

       expected: "2015-12-15T09:30:05.123+06:00 Hello World"
            got: "2015-12-15T05:30:05.123+02:00 Hello World"

@timstott
Copy link
Contributor

@bliof in what situation is a Loga event instantiated without a timestamp?

@bliof
Copy link
Contributor Author

bliof commented Nov 10, 2016

I had problems with it in #79

but here is an example

require 'logger'
require 'loga'

logger = Logger.new($stdout)
event = Loga::Event.new

logger.info(event)

Should the timestamp of the event default to current time?

@bliof bliof force-pushed the do-not-crash-in-loga-event branch from aefda88 to 020a82b Compare November 10, 2016 12:30
@timstott
Copy link
Contributor

@bliof Good example, I understand now. Overriding #to_s was a hack to output the timestamp in :simple formatter. Ultimately I want to remove the override and have the a simple formatter in Loga output the timestamp.

@timstott
Copy link
Contributor

Fixed in c794943 in #92

@timstott timstott closed this Nov 16, 2016
@timstott timstott deleted the do-not-crash-in-loga-event branch November 16, 2016 12:29
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