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

Rollbar: 2clove bug #112

Closed
johnhutch opened this issue Jun 2, 2017 · 7 comments
Closed

Rollbar: 2clove bug #112

johnhutch opened this issue Jun 2, 2017 · 7 comments
Assignees
Labels

Comments

@johnhutch
Copy link
Collaborator

ArgumentError: '2clove' Unit not recognized

@johnhutch johnhutch added the bug label Jun 2, 2017
@johnhutch johnhutch self-assigned this Jun 2, 2017
johnhutch added a commit that referenced this issue Jun 7, 2017
johnhutch added a commit that referenced this issue Jun 7, 2017
…compatible units before adding/subtracting using unit1.compatible?(unit2)
@johnhutch
Copy link
Collaborator Author

Asked question of ruby-units author for how best to handle unit validity checks.

olbrich/ruby-units#156

johnhutch added a commit that referenced this issue Jun 8, 2017
…mpat check before unit add/sub, but still needs a check for not-a-unit
johnhutch added a commit that referenced this issue Jun 14, 2017
…o ingredient model, avoids checking compatibility for non-united ingredients.
johnhutch added a commit that referenced this issue Jun 14, 2017
…th ingredient.sub not returning false if not found.
johnhutch added a commit that referenced this issue Jun 14, 2017
…try to find the source of the remaining failed test. No dice.
johnhutch added a commit that referenced this issue Jun 14, 2017
…el test. The short circuit in the sub() function in ingredient doesn't appear to actually avoid hitting the false state where the whole ingredient is removed. Not sure why. Research required on short circuiting functions via returns in ruby.
@jon-athan-hall jon-athan-hall self-assigned this Jun 20, 2017
jon-athan-hall added a commit that referenced this issue Jun 20, 2017
…stom matcher for collections, rewrote test that involved removing a recipe and not losing particular ingredients in the current grocery list
@jon-athan-hall
Copy link
Contributor

I'll review the rest of this, but take a look at the test I built (including the nifty custom matcher I researched) @johnhutch

@johnhutch
Copy link
Collaborator Author

shit, man. That custom matcher is fantastic. Any chance you saved the link where you read about it? I'd like to pinboard it.

As for the rest of the issue, review approved?

@jon-athan-hall
Copy link
Contributor

app/helpers/application_helper.rb
Is the pluralization of name backwards?

app/models/ingredient.rb

if self.unitless? || ing.unitless?
  if self.unitless? && ing.unitless?

Is this redundant? Or is it important to know if just one is unitless for the next else block? I see it twice in this file

@johnhutch
Copy link
Collaborator Author

app/helpers/application_helper.rb
Is the pluralization of name backwards?

nope.

app/models/ingredient.rb

It's important to know if just one is unitless because if EITHER is unitless, we can't try to do unit math with it.

Another way to state the logic tree would be:

If neither item is unitless, add or subtract as units.
If only one item is unitless, skip this "hit" and keep looking through the rest of the ingredients
if both items are unitless, add or subtract whole amounts.

@johnhutch johnhutch removed their assignment Jul 6, 2017
@jon-athan-hall
Copy link
Contributor

Pushed to PROD. Ready for the second round of QA.

@johnhutch
Copy link
Collaborator Author

Tested on live by adding a recipe with the following ingredient list (which originally tripped the error -- sorry, this should have been in the bug description):

5 tbsp soy sauce
4 tbsp mild or hot chili powder
3 tbsp five-spice powder
2 tbsp light muscovado sugar
2 cloves garlic, finely chopped
4 cm piece of fresh ginger, peeled and finely chopped
6 1/2 lbs boneless pork shoulder, rind removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants