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

various Improvements #2

Merged
merged 5 commits into from
Feb 22, 2020
Merged

various Improvements #2

merged 5 commits into from
Feb 22, 2020

Conversation

tripodsan
Copy link
Contributor

sorry for somewhat large PR, but I tried to keep the various changes in individual commits.

what I did:

  • added the configuration options to the README
  • move the example usage to a example directory
  • create proper mocha unit tests that build and test the output
  • set proper directories during compilation to allow correct template resolution
  • added option to exclude runtime during compilation
  • added option to specify runtime variable names
  • added option to specify module import generator

@tripodsan tripodsan requested a review from backflip February 19, 2020 03:15
@backflip
Copy link
Owner

backflip commented Feb 19, 2020

Awesome, thanks a lot!

A few notes:

  • I would prefer to ditch fs-extra and use fs.mkdir and rimraf instead. This would slightly reduce the dev dependency footprint and we could easily replace rimraf with recursive fs.rmdir at some point (available in Node 10.12+).
  • It would be great if the options in the README would link to the corresponding test.

Would you be willing to change this or should we rather merge and I update it myself? I'm planning on adding ESLint after merging this anyway, since we are starting to have enough code to warrant this, so it would be fine to take care of these minor changes as well.

@tripodsan
Copy link
Contributor Author

tripodsan commented Feb 20, 2020

@backflip I think that fs-extras is superior as it it just a think wrapper around fs and adds proper promise support.. you could also use fs.promise instead. I don't mind requiring node 10.12+ for the tests.

please go ahead and adjust the code as you whish...thanks for all your work!

@backflip backflip merged commit 4da6e38 into backflip:master Feb 22, 2020
@backflip
Copy link
Owner

Currently on my phone, but I’ll publish it on npm in the evening.

@backflip
Copy link
Owner

Published as v2.0.0

@tripodsan tripodsan deleted the improvements branch February 25, 2020 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants