Skip to content
This repository was archived by the owner on Jan 23, 2021. It is now read-only.

Conversation

@lkmorlan
Copy link
Contributor

This addresses #276.

lib/utils.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe defaults is not available here. How about making the default tempDir in index.js contain the uid? Then we won't have to change this file or the clear function at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the default tempDir was /tmp on POSIX. We can't change that name.

Copy link
Contributor

Choose a reason for hiding this comment

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

The defaults object is part of this plugin: https://github.com/lkmorlan/gulp-ruby-sass/blob/master/index.js#L22-L27. Maybe something like:

var uid = typeof process.getuid === 'function' ? process.getuid() : '';

var defaults = {
    tempDir: osTmpdir() + uid,
    ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand correctly, osTmpdir() would normally be "/tmp", it would have to be:

osTmpdir() + '/' + uid

Copy link
Contributor

Choose a reason for hiding this comment

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

True true. Go for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

My concern is that we don't ask people to pass extra arguments to the clear cache function. Although that shouldn't be used often.

Also be sure to check tests when you're done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, please use path.join instead of +.

@lkmorlan
Copy link
Contributor Author

I have made another commit which I think fixes the identified issues. Note that it changes the meaning of the tempDir configuration because gulp-ruby-sass will no longer be added to the end.

@robwierzbowski
Copy link
Contributor

Love it. Thanks!

robwierzbowski pushed a commit that referenced this pull request Nov 19, 2015
Include UID in temp dir name.
@robwierzbowski robwierzbowski merged commit 5afc369 into sindresorhus:master Nov 19, 2015
@robwierzbowski
Copy link
Contributor

I'll ping this PR when I publish a new version.

@lkmorlan
Copy link
Contributor Author

Thanks!

@stevenbower
Copy link

any chance of getting a build with this any time soon?

@robwierzbowski
Copy link
Contributor

Publishing now, should be up in a minute.

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.

3 participants