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

Use Instant for modeling timestamps #80

Closed
armanbilge opened this issue Dec 13, 2021 · 3 comments · Fixed by #98
Closed

Use Instant for modeling timestamps #80

armanbilge opened this issue Dec 13, 2021 · 3 comments · Fixed by #98
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@armanbilge
Copy link
Member

armanbilge commented Dec 13, 2021

AKA how do we feel about a scala-java-time dependency.

Pros:

  • Instant is most idiomatic representation of a timestamp

Cons:

  • Bloats generated JS

Mitigation:

@armanbilge armanbilge added the question Further information is requested label Dec 13, 2021
@bpholt
Copy link
Member

bpholt commented Dec 13, 2021

I lean towards using Instant and including the dependency. 120kb doesn't seem like a big deal for this purpose and having the idiomatic representation will be less confusing for users.

@kubukoz
Copy link
Member

kubukoz commented Dec 14, 2021

120kb doesn't seem like much in the scale of 50megs (aws limit)

@armanbilge
Copy link
Member Author

Cool, I'd say that's decided :) anyone feel free to PR an update to events with this.

@armanbilge armanbilge added enhancement New feature or request good first issue Good for newcomers and removed question Further information is requested labels Dec 14, 2021
@armanbilge armanbilge changed the title Figure out how to model timestamps Use Instant for modeling timestamps Dec 14, 2021
@armanbilge armanbilge linked a pull request Dec 16, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants