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

add UnitGroup docs; import UnitGroup in REPL #239

Merged
merged 3 commits into from
May 18, 2017
Merged

Conversation

derekmorr
Copy link
Collaborator

This adds docs for the new UnitGroup functionality. Feedback on docs would be appreciated.

Copy link
Collaborator

@cquiroz cquiroz left a comment

Choose a reason for hiding this comment

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

This looks excellent, great work

@@ -1200,6 +1306,7 @@ trait LoadRoute extends HttpService {
* Shadaj Laddad ([shadaj](https://github.com/shadaj))
* Ian McIntosh ([cranst0n](https://github.com/cranst0n))
* Doug Hurst ([robotsnowfall](https://github.com/robotsnowfall))
* Philip Axelrod ([Paxelord](https://github.com/paxelord))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I see you add @paxelord here, so no need to update PR #238

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haha. Yes. I just added a comment to #238 about this. No problem.

@@ -637,6 +637,79 @@ implicit val moneyNum = new MoneyNumeric()
val sum = List(USD(100), USD(10)).sum
```

## Unit groups
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a refresher, this generates the top README.md right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this will build the main README.


// squants automatically sorts units
usCookingUnitGroup.sortedUnits.foreach(println)
// squants.space.Teaspoons$@797a286f
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because this output includes memory locations, this part of the README will change every time tut is run.

As a future enhancement we should provide a stable toString for UnitOfMeasure that outputs the unit's name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or reduce the top README to a placeholder and put the documentation on github site, The README is getting fairly large

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I've been thinking we should split the README up into multiple smaller files. Probably just have the sbt instructions and a small quick example in the main README. I can tackle that in a later PR after this round gets merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be great to export this documentation to squants.com, That still shows version 0.6.2 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. I think updating squants.com should be the next priority after this release.

@derekmorr
Copy link
Collaborator Author

Are we ok to merge this?

Copy link
Collaborator

@cquiroz cquiroz left a comment

Choose a reason for hiding this comment

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

LGTM

@derekmorr derekmorr merged commit 519ab83 into master May 18, 2017
@derekmorr derekmorr deleted the unitgroups-docs branch May 18, 2017 17:30
@derekmorr derekmorr mentioned this pull request May 18, 2017
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