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

Default Unit #15

Merged
merged 3 commits into from
Mar 1, 2021
Merged

Default Unit #15

merged 3 commits into from
Mar 1, 2021

Conversation

andrewMacmurray
Copy link
Owner

@andrewMacmurray andrewMacmurray commented Feb 27, 2021

  • Updates api to have default millisecond unit
  • CI builds docs and examples

Resolves #8

@andrewMacmurray
Copy link
Owner Author

@lenards no rush or pressure but would be awesome if you could take a look at this one

Copy link
Contributor

@lenards lenards left a comment

Choose a reason for hiding this comment

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

I think this works out really well! 🎉
It's a good move to simplify 🙇

I was tempted to think of a name to alias (Millis -> Millis) functions to ... but that likely obscures what they are (and would make it more difficult to see that I could define my own - if I wanted). So I - perhaps - should I have omitted that 😉. Just sharing on the off-chance you had a similar thought.

@@ -9,14 +9,18 @@
"devDependencies": {
"ava": "^3.15.0",
"elm": "^0.19.1-5",
"elm-doc-preview": "^5.0.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't checked this library out until recently. It's pretty cool.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I only discovered it recently, it's a really nice feedback loop when you're working on a library!

-}
hours : Millis -> Millis
hours millis =
minutes millis * 60
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this works out really, really well.

@andrewMacmurray
Copy link
Owner Author

That's a good idea for aliasing Millis -> Millis, maybe ToUnit?

@lenards
Copy link
Contributor

lenards commented Mar 1, 2021 via email

@andrewMacmurray
Copy link
Owner Author

Gonna merge this in now, thanks for your help @lenards!

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.

Default unit
2 participants