Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

feat(gulp): Add gulp task mkdir:upload to ensure upload directory exi… #1199

Merged
merged 1 commit into from
Feb 12, 2016

Conversation

rhutchison
Copy link
Contributor

…sts.

Related #1175

@@ -202,6 +203,11 @@ gulp.task('imagemin', function () {
.pipe(gulp.dest('public/dist/img'));
});

// Make sure upload directory exists
gulp.task('mkdir:upload', function () {
return fs.mkdir('modules/users/client/img/profile/uploads', function () { });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhutchison need to handle a possible error here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error is handled with noop - how do you want it to be handled?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log it or something maybe? We basically need to notify users of it at the least.

@rhutchison rhutchison force-pushed the mkdir-upload branch 4 times, most recently from 5f0c9b8 to 4d9a444 Compare February 12, 2016 02:10
@@ -218,6 +218,15 @@ gulp.task('copyLocalEnvConfig', function () {
.pipe(gulp.dest('config/env'));
});

// Make sure upload directory exists
gulp.task('makeUploadsDir', function () {
return fs.mkdir('modules/users/client/img/profile/uploads', function (res) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick, res should be err (convention)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this convention defined?

This is a callback method for success/error and is not specifically a callback only for errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a common node convention for callbacks to return an error, so err makes more sense.

@ilanbiala ilanbiala added this to the 0.5.0 milestone Feb 12, 2016
@ilanbiala ilanbiala self-assigned this Feb 12, 2016
@ilanbiala
Copy link
Member

Also, the scope of the commit message should be gulp, because this is gulp-related, not really "core".

@rhutchison rhutchison changed the title feat(core): Add gulp task mkdir:upload to ensure upload directory exi… feat(gulp): Add gulp task mkdir:upload to ensure upload directory exi… Feb 12, 2016
@rhutchison
Copy link
Contributor Author

@ilanbiala scope is updated to reflect your feedback

@rhutchison
Copy link
Contributor Author

@ilanbiala LGTY?

@rhutchison
Copy link
Contributor Author

Closing/Opening to produce clean build. One node timed out.

@rhutchison rhutchison closed this Feb 12, 2016
@rhutchison rhutchison reopened this Feb 12, 2016
ilanbiala added a commit that referenced this pull request Feb 12, 2016
feat(gulp): Add gulp task mkdir:upload to ensure upload directory exi…
@ilanbiala ilanbiala merged commit efed847 into meanjs:master Feb 12, 2016
@rhutchison rhutchison deleted the mkdir-upload branch February 13, 2016 02:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants