Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix# 143 - EPERM error in Windows #144

Merged
merged 4 commits into from
Sep 9, 2015
Merged

Fix# 143 - EPERM error in Windows #144

merged 4 commits into from
Sep 9, 2015

Conversation

sjmcdowall
Copy link
Contributor

This is just a simple kludge to remove the error when demeteorizing on Windows with an older node version. Seems to work in my testing.

@fiveisprime
Copy link
Collaborator

This doesn't exactly do what you want it to... Just replace the comment block
and the unlink call with

Fs.chmodSync(packagePath, '0644');

You'll have to update the tests to reflect this change as well. This is a much
better solution to the file permissions. 😄

@sjmcdowall
Copy link
Contributor Author

Well -- it does do exactly what I want .. but maybe it's not the cleanest. :) I was trying to minimize the code flow and only for the case where we failed on Windows. Are you saying you'd rather always just do the chmod .. and no unlink is even needed? Well that's cool.

Also, being new to this -- how do I run the tests "locally" before I commit to make sure they pass?

Cheers

@fiveisprime
Copy link
Collaborator

Yeah, there's no need to remove the file if the permissions are changed. 👍

Run npm test to run the tests locally. You'll need to replace the unlinkSync
mock in the unit test with a chmodSync mock. That should do it for the tests.

@sjmcdowall
Copy link
Contributor Author

Thanks -- did exactly that and now everything seems to be working fine.

@fiveisprime
Copy link
Collaborator

Just a couple of things, convert your tabs to spaces, change your quotes to
single quotes, and remove the extra space around those calls.

@sjmcdowall
Copy link
Contributor Author

OK -- but I rather LIKE the spaces around the calls .. IMHO -- easier to read.. but that's me.. :)

I know I should have used Sublime to make the changes and not vi! LOL ..

@fiveisprime
Copy link
Collaborator

One more thing, the second call to chmodSync isn't necessary since the file
isn't being overwritten. Also, that call happens after the done callback is
passed off.

fiveisprime pushed a commit that referenced this pull request Sep 9, 2015
Fix# 143 - EPERM error in Windows
@fiveisprime fiveisprime merged commit 2ca819d into XervoIO:master Sep 9, 2015
@sjmcdowall
Copy link
Contributor Author

Well that was easy! HA! :)

@fiveisprime
Copy link
Collaborator

👍 Thanks for the contribution!

@sjmcdowall
Copy link
Contributor Author

Thanks for walking me through it -- obviously once you saw the original code you could have had it all done in about 2 minutes and not 3 more commits! :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants