testdouble / scripty

Because no one should be shell-scripting inside a JSON file.
MIT License
963 stars 23 forks source link

Support singular 'script' dirname convention #72

Closed jasonkarns closed 5 years ago

jasonkarns commented 5 years ago

I can't find any documentation or any source that finds either of 'scripts' or 'script' to be more common in practice.

Absent any data, I lean only on anecdotal experience when I find 'script' to be more common in the wild than 'scripts'.

To further defend this, GitHub has blogged their "Scripts to Rule Them All" pattern, and for it they use 'script' singular.

This change adds 'script' (and 'script-win') as fallbacks if the plural version does not exist.

This should be a semver minor change.

Any users who may incidentally already have both scripts and script directories, scripts will continue to be preferred (as long as it exists). The singular 'script' is only used as a fallback when scripts does not exist.

The same is true for Windows users. scripts-win is preferred if both scripts-win and script-win exist.

The only users who may experience an issue are Windows users who do not have a scripts-win directory but do have a script-win directory, for some reason. In this case, what would have fallen back to scripts will now be preempted by script-win.

closes #52

searls commented 5 years ago

seems legit

jasonkarns commented 5 years ago

@searls latest commit refactors the tests to stub fs.existsSync instead of stubbing process.cwd(). The latter required there being fixture directories such that the existsSync calls would pass/fail accordingly. I think the stubbing of existsSync is more clear since the tests are now explicit about directories existing or not (as opposed to that knowledge living in the fixtures).

Also adds a test to cover the scripts -> script fallback behavior for modules as well.

I've removed the Do Not Merge flag, so it's merge-ready if tests/code are 👍

searls commented 5 years ago

Skimmed and merged