Skip to content

Stop using deprecated process.umask()#28

Merged
sindresorhus merged 1 commit intosindresorhus:masterfrom
coreyfarrell:no-process-umask
Apr 22, 2020
Merged

Stop using deprecated process.umask()#28
sindresorhus merged 1 commit intosindresorhus:masterfrom
coreyfarrell:no-process-umask

Conversation

@coreyfarrell
Copy link
Copy Markdown
Contributor

Fixes #27


Note readme.md and index.d.ts both still reference 0o777 & (~process.umask()) as the default value to mode. I'm not sure the best way to update these two places.

@sindresorhus
Copy link
Copy Markdown
Owner

Just remove the (~process.umask()) part?

@coreyfarrell
Copy link
Copy Markdown
Contributor Author

So you don't think we need to mention that the mode is still filtered by the process umask?

Side note I have no idea what the issue with node 10 on Windows is. I don't have access to Windows (outside of Travis) so I'm not sure how to troubleshoot this.

@coreyfarrell
Copy link
Copy Markdown
Contributor Author

Hmm I guess since https://nodejs.org/dist/latest/docs/api/fs.html#fs_fs_mkdir_path_options_callback just says the default is 0o777 so I doing the same here probably makes sense.

@sindresorhus sindresorhus merged commit b5a98ef into sindresorhus:master Apr 22, 2020
@sindresorhus
Copy link
Copy Markdown
Owner

I don't know about the Windows issue either. And I have no ability to debug it further.

@sindresorhus
Copy link
Copy Markdown
Owner

Thanks for fixing this 👍

@coreyfarrell coreyfarrell deleted the no-process-umask branch April 23, 2020 14:30
sindresorhus pushed a commit that referenced this pull request Apr 25, 2020
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.

Stop using process.umask()

2 participants