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

Implemented overwrite option in dest() #59

Merged
merged 3 commits into from
Feb 23, 2015
Merged

Implemented overwrite option in dest() #59

merged 3 commits into from
Feb 23, 2015

Conversation

slife
Copy link
Contributor

@slife slife commented Feb 23, 2015

This is in reference to: #30

It seemed like the overwrite option was stubbed out, but the wx flag was never utilized in the fs operations. This commit wires it up, handles the error catch scenario and adds some tests. Let me know what you think!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 96.65% when pulling 5572793 on derekslife:master into 2dfe290 on wearefractal:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 96.65% when pulling 32fc5c8 on derekslife:master into 2dfe290 on wearefractal:master.

@@ -31,7 +31,9 @@ function writeContents(writePath, file, cb) {
}

function written(err) {
if (err) {
var ignorableError = (err && err.code === 'EEXIST' && file.flag === 'wx');
Copy link
Member

Choose a reason for hiding this comment

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

canIgnoreError or isErrorFatal would be more descriptive imo

@yocontra
Copy link
Member

Very clean PR - one minor nitpick and I am +1 on merging this. Thanks! 🌴

@coveralls
Copy link

Coverage Status

Coverage increased (+0.11%) to 96.73% when pulling c4750b2 on derekslife:master into 2dfe290 on wearefractal:master.

yocontra pushed a commit that referenced this pull request Feb 23, 2015
Implemented `overwrite` option in `dest()`
@yocontra yocontra merged commit 6d79529 into gulpjs:master Feb 23, 2015
@slife
Copy link
Contributor Author

slife commented Feb 23, 2015

Thanks!

@yocontra
Copy link
Member

Available on master - will be published in the 4.0 release

@jlukic
Copy link

jlukic commented Mar 30, 2015

👍

@phated
Copy link
Member

phated commented Jul 12, 2016

@Narretz comments like that are not welcome here. Feel free to use the module directly.

@gulpjs gulpjs locked and limited conversation to collaborators Jul 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants