Replacing an invalid return statement with a valid exit code and allowing gulp to be run anywhere within a project.

[jsdevel]

No description provided.

[yocontra]

Couple of comments

1. Not a fan of doing the chdir within getGulpfile - getGulpfile should have no side effects that way it can be tested easily
2. What change was introduced to getGulpfile other than adding the chdir? The whole function was completely rewritten and I'm not sure why - I prefer the functional style it used before

+1 on the return statement fix

[jsdevel]

1. With the change, I can gulp in any directory within my project, not just the directory where my gulpfile.js resides.
2. A side effect of the previous item is: relative paths within my project that depended on CWD are now broken.
3. The chdir considers the previous two points and prevents me from having to specify absolute paths everywhere within my code.
4. Testing shouldn't be effected at all, in fact it should be enhanced as the CWD of gulpfile.js is always going to be the project root.
5. require.extensions has been deprecated http://nodejs.org/api/globals.html#globals_require_extensions

[yocontra]

gulpfile locating: I would prefer if we used a 3rd party module for walking up to find the gulpfile (grunt uses findup-sync for this)

Testing/chdir: Don't do a chdir in the function - do it before the gulpfile is loaded. I want the getGulpFile function to have no side effects.

As for require.extensions being deprecated:

Since the Module system is locked, this feature will probably never go away. However, it may have subtle bugs and complexities that are best left untouched.

If they ever remove it or an alternative arises I will revisit changing it. The changes you put in break support for coffee-script (which I personally use).

[jsdevel]

+1 for findup-sync

You're still doing a chdir within a function though 👯

[yocontra]

@jsdevel the function name is what determines if it should have side effects. get(something) should not set something externally as well. set(something) or load(something) is perfectly fine to have some side effects in

[jsdevel]

@contra I disagree about set and load in relation to external side effects. setProperties() would be strange if it did a chdir.

I think a better option would be to scope chdir within the program instead of within a function, similar to the following:

if (!gulpFile) {
  cliGulp.log(chalk.red('No Gulpfile found'));
  process.exit(1);
} else {
  process.chdir(path.dirname(gulpFile));
}

I'm happy with your solution though. Now I can gulp away 👍

[yocontra]

2.4 is published let me know if your problem is still unsolved