Skip to content

Improve virtual env detection + #807#831

Merged
MikhailArkhipov merged 29 commits into
microsoft:masterfrom
MikhailArkhipov:807
Feb 20, 2018
Merged

Improve virtual env detection + #807#831
MikhailArkhipov merged 29 commits into
microsoft:masterfrom
MikhailArkhipov:807

Conversation

@MikhailArkhipov

Copy link
Copy Markdown

Fixes #807

  • Implemented detection of virtual env by running Python code
  • Removed detect from provider since we detect ve universally at the manager lever
  • provider returns venv name
  • remove duplicates from interpreter selector by checking for aliases

const env = results.find(items => items.result === true);
return env ? env.env : undefined;
});
public async detect(pythonPath: string): Promise<string> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't this function return a boolean (the string returned implies a boolean)

@MikhailArkhipov MikhailArkhipov Feb 20, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I used to return identifier which was pair of name and the interpreter type. Not it is just name. Perhaps we should rename this to getEnvironmentName

@codecov

codecov Bot commented Feb 20, 2018

Copy link
Copy Markdown

Codecov Report

Merging #831 into master will decrease coverage by 0.3%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #831      +/-   ##
==========================================
- Coverage   63.76%   63.46%   -0.31%     
==========================================
  Files         260      258       -2     
  Lines       11864    11845      -19     
  Branches     2103     2104       +1     
==========================================
- Hits         7565     7517      -48     
- Misses       4291     4320      +29     
  Partials        8        8
Impacted Files Coverage Δ
src/client/common/platform/types.ts 100% <ø> (ø) ⬆️
src/client/common/configSettings.ts 87.17% <ø> (ø) ⬆️
src/client/interpreter/virtualEnvs/types.ts 100% <ø> (ø) ⬆️
src/client/interpreter/index.ts 63.63% <0%> (+0.71%) ⬆️
src/client/common/platform/fileSystem.ts 78.26% <0%> (-12.22%) ⬇️
...rpreter/locators/services/baseVirtualEnvService.ts 75% <0%> (ø) ⬆️
src/client/interpreter/virtualEnvs/index.ts 100% <100%> (ø) ⬆️
src/client/interpreter/contracts.ts 100% <100%> (ø) ⬆️
src/client/interpreter/serviceRegistry.ts 97.82% <100%> (-0.18%) ⬇️
src/client/interpreter/display/index.ts 100% <100%> (ø) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6eabde4...7f3e4fa. Read the comment docs.

@MikhailArkhipov MikhailArkhipov merged commit 7894ae6 into microsoft:master Feb 20, 2018
@MikhailArkhipov MikhailArkhipov deleted the 807 branch February 20, 2018 22:05
@lock lock Bot locked as resolved and limited conversation to collaborators Jul 31, 2019
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.

Filter out aliases in interpreter list?

2 participants