search

persvr / rql

Resource Query Language
http://www.persvr.org/
268 stars 49 forks source link

Typo in reducer ; aggregation do not works #57

Closed NicolasJacob closed 9 years ago

NicolasJacob commented 9 years ago

require("rql/js-array").executeQuery("min(price)", {}, data) ReferenceError: resul is not defined at Array. (/node_modules/rql/js-array.js:323:10)

neonstalwart commented 9 years ago

@NicolasJacob interested in opening a PR to add a test and fix this. i realize the fix is a simple matter of moving a semi-colon one character but obviously we don't have a test that touches this code something that uses that code would be better than nothing.

if you're able to contribute a test, please make sure to sign the cla

wshager commented 9 years ago

@neonstalwart Sorry, but really? I'll do it then.

neonstalwart commented 9 years ago

this issue demonstrates that the tests are completely inadequate since something so blatantly wrong is even in the codebase. so i'm determined to use this as an opportunity to improve the tests. simply moving the semi-colon is not a complete solution.

this is open source and i just don't have time right now. if someone else has the time to do it now, it will get done sooner than waiting on me. often, the person reporting an issue is already in a context where it is not a huge burden for them to contribute. if someone else does not have time, they are also not compelled to do it and i'll get to it when i do have time.

wshager commented 9 years ago

I understand your point, but there are issues open from 5 years ago. Let's not accumulate more for the sake of a being correct.

wshager commented 9 years ago

I updated the test to include sum and aggregate. #63