-
Notifications
You must be signed in to change notification settings - Fork 31
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
Drop uuid
and just use native mkdtemp
#122
Conversation
uuid
and just use native mktempuuid
and just use native mkdtemp
@@ -603,7 +602,7 @@ FirefoxProfile.prototype._installExtension = function (addon, cb) { | |||
self = this; | |||
|
|||
if (addon.slice(-4) === '.xpi') { | |||
tmpDir = this._createTempFolder(addon.split(path.sep).slice(-1)); | |||
tmpDir = this._createTempFolder(addon.split(path.sep).pop()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slice
returned an array. It worked before because _createTempFolder
was concatenating this with +
@@ -152,7 +151,7 @@ function FirefoxProfile(options) { | |||
this.profileDir = opts.destinationDirectory || this._createTempFolder(); | |||
} else { | |||
// create copy | |||
var tmpDir = opts.destinationDirectory || this._createTempFolder('-copy'); | |||
var tmpDir = opts.destinationDirectory || this._createTempFolder('copy-'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to change this to be a prefix instead of suffix due to how the API works.
This path is meant to be temporary and hidden from the user, so I'm guessing it was implemented only to improve debugging but isn't fundamental.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here is to not alter an existing profile directory (if provided in options.profileDir
).
@fregante I released v4.2.0 earlier today 🎉 ! |
Node has had a way to create temporary directories since v5: https://nodejs.org/api/fs.html#fs_fs_mkdtempsync_prefix_options
Sorry for the noise in this PR. It took me a while to figure out why it was refusing to work on Travis