Skip to content

tempdir: use os.tmpDir when possible#67

Merged
arturadib merged 2 commits intoshelljs:masterfrom
stephenmathieson:os-dot-tmpdir
Jul 16, 2013
Merged

tempdir: use os.tmpDir when possible#67
arturadib merged 2 commits intoshelljs:masterfrom
stephenmathieson:os-dot-tmpdir

Conversation

@stephenmathieson
Copy link
Copy Markdown

use node's native tmpDir when possible, but fallback to the python's algorithm in old (really old) versions of node

test/tempdir.js Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm I think this test is unnecessary -- from the user's viewpoint, it doesn't matter what exactly the temp dir, all they want is a writeable temp directory.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point; I'll revert this tonight.

On Jun 11, 2013, at 3:27 PM, Artur Adib wrote:

In test/tempdir.js:

@@ -21,6 +22,10 @@ shell.mkdir('tmp');
//

var tmp = shell.tempdir();
+// node 0.8+
hmm I think this test is unnecessary -- from the user's viewpoint, it doesn't matter what exactly the temp dir, all they want is a writeable temp directory.


Reply to this email directly or view it on GitHub.

@stephenmathieson
Copy link
Copy Markdown
Author

@arturadib thoughts? should i close?

@arturadib
Copy link
Copy Markdown
Collaborator

sorry Stephen, I'm on paternity leave and not online very much. thanks that looks great!

arturadib added a commit that referenced this pull request Jul 16, 2013
tempdir: use `os.tmpDir` when possible
@arturadib arturadib merged commit 3935725 into shelljs:master Jul 16, 2013
@stephenmathieson
Copy link
Copy Markdown
Author

no problem and congrats! :)

@stephenmathieson stephenmathieson deleted the os-dot-tmpdir branch July 16, 2013 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants