Closed imacneill closed 8 years ago
Thanks so much for the fix. One request: Could you add a test in https://github.com/esjewett/reductio/blob/master/test/alias.spec.js that fails without the fix and passes with it? Sounds like you'd just need to build a reducer as mentioned in the issue you filed and then test it. If you need help on that, please let me know.
Hi, I added a second alias to your test example in the suite and added a new spec testing the second alias. I tested it with the pre-fixed reductio.js and the post-fixed version. It fails the former and passes the latter.
New to the way github does things. I committed and pushed the test changes to the same branch on my fork as in the initial pull request. I see the new commit message above, so I hope those steps were enough to update the pull request. If not, let me know.
Thanks for pointing me towards Jasmine. This seems like a really useful tool.
The test is failing because the new test conditions are incorrect. Can you fix? You are correct to just push to the existing branch you used to open the pull request. Thanks!
BTW, you can run the tests locally with npm test
Sorry, not ignoring this. Have just been too busy at work to do anything else. I'll try to get to it soon.
No worries. Whenever you get the chance. I appreciate the help!
Looks like I figured it out now. I just didn't understand your workflow (and if it's a common one, I'm just ignorant of it). I modified the reductio.js in the main directory, but when I looked through the test suit, I realized that the test was copying it over from the src/ directory to the main directory during the step where it produced the min version. So the updated test was failing because the new code was being overwritten by the old code, and the min file produced was made from the old min file.
I did not commit the new min file. I wasn't sure if that was something you wanted to do yourself and not worry about it during the merging.
By the way, thanks for being patient with me. This has been a nice introduction to number of useful tools. Please let me know if I need to do anything else on this issue.
Hi - no problem with figuring out how it works. Everyone has to go through that :-)
So here's what you need to do. Sorry I wasn't more clear about it before. I'm assuming you have NPM and Gulp.js installed, and you've run npm install
in the base directory of your Reductio folder.
gulp all
in the same directory as the Gulp file.Until step 5, I'm happy to help you work through any problems on this issue thread. Generally speaking, only change files in src/ or test/. I apologize that I didn't realize you'd changed the main generated reductio.js file earlier.
All of that seemed to have worked fine.
For my new pull request, do you want me to include the reductio.js and reductio.min.js that come out of the gulp all command? I'm assuming yes. Just want to double check before I open it.
Awesome! Either way is fine. If you want to be super-clean, don't include them, but personally I don't care.
Handled in https://github.com/esjewett/reductio/pull/32, so I'm withdrawing this one.
wrapped the function in another function and used the prop as an argument to the function to evaluate it in place for the return function.
See my open issue for a much longer explanation: https://github.com/esjewett/reductio/issues/27