Skip to content
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

process#findExecutable() should not use sync fs #70577

Closed
bpasero opened this issue Mar 15, 2019 · 3 comments
Closed

process#findExecutable() should not use sync fs #70577

bpasero opened this issue Mar 15, 2019 · 3 comments
Assignees
Labels
debt Code quality issues tasks Task system issues
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Mar 15, 2019

Looks like win32.findExecutable is being used from the renderer and has 3 occurrences of fs.existsSync() that should use the async variant instead. It should be possible to just return a promise from this method.

Feel free to move forward if you are not the owner of this method, I just went by git history.

@bpasero bpasero added the debt Code quality issues label Mar 15, 2019
@bpasero
Copy link
Member Author

bpasero commented Mar 17, 2019

@dbaeumer looks like I filed this in 2017 already (#34425). Is there a technical problem resolving this?

@dbaeumer
Copy link
Member

No technincal problem. I even think that this has no super high priority right now since the whole code needs to move out of the renderer anyways. This is independent of whether it is sync or async.

@bpasero
Copy link
Member Author

bpasero commented Mar 18, 2019

@dbaeumer oh ok, if it moves out of the renderer into the extension host than its less critical. Though I still think sync API should not be used if possible (I know we are not very good in other processes besides the renderer).

@alexr00 alexr00 added this to the Backlog milestone Mar 19, 2019
@alexr00 alexr00 added the tasks Task system issues label Mar 19, 2019
@alexr00 alexr00 modified the milestones: Backlog, June 2019 Jun 5, 2019
@alexr00 alexr00 closed this as completed in f3086a6 Jun 5, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues tasks Task system issues
Projects
None yet
Development

No branches or pull requests

3 participants