Event default timestamp

[armstrys]

Changed the Event default value for created_at to None and set time stamp within __init__. Also added a test file for events.

[callebtc]

LGTM apart from the nit

[armstrys]

Wow. I had never seen that solution. That’s going to save me a lot of text in my other work. Thanks :)

Also I wasn’t sure how to link it here, but this request will close issue 23.

[jeffthibault]

I'm not clear on why this change is needed. I think having the created_at time default to the current time is both logical and convenient.

If you want to use a custom time, you can do something similar to my suggestion in #23

[armstrys]

I also left a comment on #23, but wanted to leave one here to make sure it is clear. I agree that using the current time as a default is the most logical approach, but I don't think the current implementation behaves as expected - I see this PR as a bug fix as opposed to a new feature.

[jeffthibault]

Thanks, I was able to reproduce the bug. LGTM

[kdmukai]

@armstrys Can you add a follow-up update to document in the test what the test is addressing?

In general, each test should explain itself (""" should use time of instantiation as...""") and in this case I'd also reference the Issue num/link as further explanation.

[kdmukai]

Also I think a better test approach would be to create two Event instances w/a delay between them and verify that they have different created_at times.

[armstrys]

Thanks for the feedback - That makes sense to me. Working on an update with both of those changes. Relatively new to contributions... can I push that update back up to this pull request or do I need to open a new one?

[kdmukai]

New PR. But reference this one in the PR notes. If you write #24 (just as normal text), github will autolink it: #24.

[armstrys]

Okay, new PR is up and this all is making more sense. Thanks to both of you for your patience and help!