-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Coffee unit #33
Coffee unit #33
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #33 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 80 88 +8
Branches 16 18 +2
=========================================
+ Hits 80 88 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes I commented.
Do you think we can change this parameter to unit
instead of making it specific to coffee
?
I'd guess you want to implement some similar thing for water
too. Having two parameter to control coffee and water seems over-engineering to me.
mycoffee/params.py
Outdated
"oz": {"name": "ounce", "rate": 0.03527396195}, | ||
"lb": {"name": "pound", "rate": 0.00220462262185}, | ||
"mg": {"name": "milligram", "rate": 1000}, | ||
"kg": {"name": "kilogram", "rate": 0.001}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"kg": {"name": "kilogram", "rate": 0.001},
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 444b4eb
At first, I had planned to add a single argument for the unit, but some of the units are specific to coffee or water—for example, " Approximate number of beans"—so we should add them separately. |
Reference Issues/PRs
#13
What does this implement/fix? Explain your changes.
show_coffee_units_list
function added--coffee-unit
argument addedREADME.md
updatedAny other comments?