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

feature: add unless exists option to add operation #27

Merged
merged 2 commits into from Mar 2, 2018

Conversation

Projects
None yet
2 participants
@tobmaster
Contributor

tobmaster commented Mar 2, 2018

Added a feature to the add operation to only execute unless the file already exists.

Motivation for this feature

When generating code it is necessary to not only generate the new code but also use injections to "register" it in other parts of the application. This works fine by using the inject feature.
But it only works correctly when the target file with the base structure is already there, which is not ensured.
In these cases someone would like to write a template that ensures the scaffold is produced before and do the injection in another template for the actual "registration".

For example:
Assume you have the following application structure:

app
├ modules
  ├ my_module
    ├ controllers
      ├ controllerA.js
    ├ tests
     ├ suite.js
     ├ controllerA.spec.js

Its easy to build a generator that creates a new "controllerB" in my_module/controllers and its test int my_module/tests and also inject the controllerB.spec.js registration in the suite.js.

But maybe the my_module doesn't have tests and a suite.js file yet. I would need to create a completely new generator just so that i have one that also generates the suite.js.

Therefore I could use a unless_exists to only run a template for scaffolding the suite.js when it doesn't exist, but also reuse all the other templates no matter if its a brand new (aka empty) module or I want to make an addition to an existing module.

Docs

The docs are missing yet. I would write a doc part for it when you are fine with the feature and its not against the project goals.

@jondot

Awesome idea, and clean implementation, thanks!
I only had one note here but otherwise I'm ready to merge

if (!to || inject) {
return
}
const absTo = path.join(cwd, to)
const shouldNotOverwrite = (typeof unless_exists !== 'undefined') && unless_exists === true

This comment has been minimized.

@jondot

jondot Mar 2, 2018

Owner

I'm wondering why not this (am I missing something?):

unless_exists !== undefined

This comment has been minimized.

@tobmaster

tobmaster Mar 2, 2018

Contributor

Shouldn't be an issue in this case probably. But I normally do it that way to prevent errors from beeing triggered. Like defensive coding ;)
I can quickly change it if you mind it

This comment has been minimized.

@jondot

jondot Mar 2, 2018

Owner

Yup only if it's quick and you don't mind, please change it. Only because my first reaction was "did I miss a best practice that I didn't get a chance to learn?" makes it a bit unsurprising.

tobmaster
@tobmaster

This comment has been minimized.

Contributor

tobmaster commented Mar 2, 2018

Fixed. Thanks for the quick responding :)

@jondot

This comment has been minimized.

Owner

jondot commented Mar 2, 2018

Awesome stuff! merging!

@jondot jondot merged commit cb7d27c into jondot:master Mar 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jondot

This comment has been minimized.

Owner

jondot commented Mar 2, 2018

I've taken the liberty to add the appropriate documentation to the add op. There should be a new version and a new website pushed once CI finishes doing its magic.

@tobmaster

This comment has been minimized.

Contributor

tobmaster commented Mar 2, 2018

@jondot youre welcome. Thanks for the quick handling and taking care of the doc part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment