thinkthroughmath / ttm-coffeescript-math

Coffeescript math library used in Think Through Math
MIT License
1 stars 0 forks source link

[#8771535]Prove that the evaluation is valid from ttm-math #7

Closed ninjapanzer closed 9 years ago

ninjapanzer commented 9 years ago

What am I?

Fixes a index error when dealing with square root operations. All operations after the sq_root would fail because the lookup indices would be off by three. This is the number of added expressions needed to complete the sq_root in literal expression {"root", 9, 2}

QA

I am having issues writing tests for this because I think there might be a bug in the refinement associated with multiplication

After merge

make a release for 0.3.2

robwierzbowski commented 9 years ago

Does this affect only calc? Looks good to me.

joelmccracken commented 9 years ago

example test looks good.

ninjapanzer commented 9 years ago

Its not. I got cornfused between the javascript-calculator and this

joelmccracken commented 9 years ago

Oh derp. Yeah, button pressing == calculator/eq builder specific

robwierzbowski commented 9 years ago

I can't remember if the square works the same way in eq builder as in the calc. If it does (it probably does) this is :+1:.

ninjapanzer commented 9 years ago

its a boondoggle

ninjapanzer commented 9 years ago

So this evaluation does work with ttm math on all operations

So this test just proves that and passes. Moving on the thinkthroughmath/javascript-calculator

ninjapanzer commented 9 years ago

This is ready to go it just proves the problem isn't here

ttm-jenkins commented 9 years ago

Code review assigned to dev(s): @seejee, @MandaBrown

MandaBrown commented 9 years ago

This looks good to me. :+1:

carols10cents commented 9 years ago

I'm stealing this CR from @seejee... I love these small, focused commits!!!! :heart: :heart: :heart:

Is there anything for QA to do here or will they need to use javascript-calculator and/or lesson-player to be able to see a difference?

ninjapanzer commented 9 years ago

correct this can be merged anytime and no one will be the wiser

carols10cents commented 9 years ago

well then!