ianmackenzie / elm-units

Simple, safe and convenient unit types and conversions for Elm
https://package.elm-lang.org/packages/ianmackenzie/elm-units/latest/
BSD 3-Clause "New" or "Revised" License
85 stars 14 forks source link

Add Molarity module #44

Closed lenards closed 5 years ago

lenards commented 5 years ago

Relates to #6. Applies feedback from #42.

Work Done

In SubstanceAmount module:

In Molarity module:

What Is Missing?

molesPerLiter, inMolesPerLiter

Because Molarity is defining the value as a Rate, and there is not direct access to a numerator and denomerator, I am not sure what that because what to define molesPerLiter given molesPerCubicMeter and/or decimolesPerLiter.

A good example in the module comment

My lack of familiarity with concentration, or amount-of-substance of concentration, means that I'm not sure what a good usage example would be to include in the module comment. I would appreciate thoughts, suggestions, etc.

Thank you

I appreciate the feedback on the draft pull request, see #42. Thank you for your time in considering this contribution.

lenards commented 5 years ago

@ianmackenzie / Ian - can you tag this with Hacktoberfest label?

lenards commented 5 years ago

Checking the failure.

lenards commented 5 years ago

onSave hook in my editor was not applying elm-format. Apply elm-format, squash changes in rebase to avoid any extraneous commits in the pull request.

ianmackenzie commented 5 years ago

Just added a section to the introductory docs describing how a Molarity value can be worked with as a rate - is this roughly what you meant by "A good example in the module comment"?

Would probably be even better to have some full examples with actual values, but then that should probably be done for all other similar modules as well (Speed, Pressure, Current etc.).

lenards commented 5 years ago

I appreciate the additional commits. Also want to pre-apologize for not getting those bits in the branch before the PR was created.

ianmackenzie commented 5 years ago

No, not at all! I quite enjoy working on PRs collaboratively, I think it works quite well.

I think this is pretty much ready to merge, but one last thing - can you add yourself to the AUTHORS file? 🙂

lenards commented 5 years ago

Added myself 🙇

lenards commented 5 years ago

Thank you Ian!

ianmackenzie commented 5 years ago

Looks good, thanks!