Conversation
Tzrlk
commented
Apr 15, 2023
- Reducing code duplication
- Using import destructuring.
- Ignoring integration test file output.
- Changing package 'test' script to not use hashbang execution.
- Explicitly requiring jake rather than relying on global import
- Fixing integration tests to run on Windows.
|
I'm aware this set of changes is less surgical than it could be, so if there's anything you'd like me to pull back on, please let me know. I really wanted to be able to get the tests running properly in my windows environment before I started rewriting more things. |
|
I also note that the travis build specifies node 8, 10, and 12. These changes will definitely not pass those builds. I might look into improving the test runner to execute the jake cli against different nodejs versions, so compatibility with older runtimes can be maintained with transpiling and/or conditional shims. |
|
This looks amazing! However, I noticed a bunch of semicolon-less lines in your changeset, and reaalized the following:
I have fixed both of these issues. The conflicts in your PR are doubtless just collisions where I am not super concerned about the older versions of Node. Jake has been around since Node v2, and sometimes things in the codebase take a while to catch up to reality. |
* Reducing code duplication * Using import destructuring. * Ignoring integration test file output. * Changing package 'test' script to not use hashbang execution. * Explicitly requiring jake rather than relying on global import * Fixing integration tests to run on Windows. * Fixing no-sparse-arrays eslint error. * Fixing assertion param ordering * Making test tmp dir deletion/creation easier. * Fixing windows jake batch script. * Ensuring `jake lint` passes. * Removed some unused params.
|
I'm super happy I got a positive response to this; thanks a ton. Do you have a preference for Travis? Or would Github actions do just as well? |