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.

Sign up for GitHub

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

Jump to bottom

Rename resolveShellEnv to getResolvedShellEnv #137923

Closed
aeschli opened this issue Nov 26, 2021 · 1 comment
Closed

Rename resolveShellEnv to getResolvedShellEnv #137923

aeschli opened this issue Nov 26, 2021 · 1 comment
Assignees
@bpasero
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Milestone
November 2021

Comments

@aeschli
Copy link
Contributor

aeschli commented Nov 26, 2021

When I investigated for #137900, I stumbled across https://github.com/microsoft/vscode/blob/main/src/vs/platform/request/node/requestService.ts#L70.

I worried that resolveShellEnv would resolve the env on every request. But in fact resolveShellEnv uses a cached promise so all good.

To reflect that I suggest to rename resolveShellEnv to getResolvedShellEnv
@bpasero bpasero added the debt Code quality issues label Nov 26, 2021
@bpasero bpasero added this to the November 2021 milestone Nov 26, 2021
@bpasero
Copy link
Member

bpasero commented Nov 26, 2021

I renamed it. Note that if the promise is rejected, request service and pty service will log an error to the log file on each time the method is used.

//cc @chrmarti @Tyriar

chrmarti added a commit that referenced this issue Nov 29, 2021
@chrmarti
Log only once (#137923)
39332e7
guibber pushed a commit to guibber/vscode that referenced this issue Nov 30, 2021
@bpasero
Rename resolveShellEnv to getResolvedShellEnv (fix microsoft#137923)
346e9b1
guibber pushed a commit to guibber/vscode that referenced this issue Nov 30, 2021
@chrmarti
Log only once (microsoft#137923)
bc47607
@github-actions github-actions bot locked and limited conversation to collaborators Jan 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bpasero bpasero

Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Milestone
November 2021
Development

No branches or pull requests
3 participants
@bpasero @aeschli and others