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

Added ability to manually enter a path to interpreter in the select interpreter dropdown #11227

Merged
merged 7 commits into from
Apr 24, 2020

Conversation

karrtikr
Copy link

@karrtikr karrtikr commented Apr 17, 2020

For #216

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • The wiki is updated with any design decisions/details.

@karrtikr karrtikr force-pushed the enterInterpreterPath branch from 75ea001 to 89601eb Compare April 17, 2020 14:41
@karrtikr karrtikr force-pushed the enterInterpreterPath branch from 89601eb to f7f3ade Compare April 17, 2020 16:07
@karrtikr karrtikr marked this pull request as draft April 17, 2020 16:41
@karrtikr karrtikr marked this pull request as ready for review April 20, 2020 19:26
@karrtikr karrtikr force-pushed the enterInterpreterPath branch from 05025f0 to e65ec36 Compare April 20, 2020 19:29
@karrtikr karrtikr force-pushed the enterInterpreterPath branch from a8aa661 to e26b456 Compare April 20, 2020 19:39
package.nls.json Outdated Show resolved Hide resolved
package.nls.json Show resolved Hide resolved
src/client/common/utils/multiStepInput.ts Outdated Show resolved Hide resolved
Copy link

@kimadeline kimadeline left a comment

Choose a reason for hiding this comment

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

Same comments as Karthik above

@codecov-io
Copy link

codecov-io commented Apr 22, 2020

Codecov Report

Merging #11227 into master will decrease coverage by 1.39%.
The diff coverage is 79.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11227      +/-   ##
==========================================
- Coverage   61.45%   60.05%   -1.40%     
==========================================
  Files         596      616      +20     
  Lines       32874    37365    +4491     
  Branches     4659     5614     +955     
==========================================
+ Hits        20203    22441    +2238     
- Misses      11654    14329    +2675     
+ Partials     1017      595     -422     
Impacted Files Coverage Δ
src/client/interpreter/configuration/types.ts 100.00% <ø> (ø)
...erpreterSelector/commands/setShebangInterpreter.ts 28.57% <28.57%> (ø)
src/client/common/utils/multiStepInput.ts 70.53% <42.85%> (-1.85%) ⬇️
...uration/interpreterSelector/interpreterSelector.ts 85.18% <85.18%> (ø)
...n/interpreterSelector/commands/resetInterpreter.ts 88.23% <88.23%> (ø)
...configuration/interpreterSelector/commands/base.ts 91.30% <91.30%> (ø)
...ion/interpreterSelector/commands/setInterpreter.ts 94.44% <94.44%> (ø)
src/client/common/utils/localize.ts 95.48% <100.00%> (+0.18%) ⬆️
src/client/interpreter/serviceRegistry.ts 100.00% <100.00%> (ø)
.../datascience/jupyter/interpreter/jupyterCommand.ts 40.24% <0.00%> (-14.93%) ⬇️
... and 207 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 d0d356f...67f1b4d. Read the comment docs.

@karrtikr karrtikr requested a review from kimadeline April 22, 2020 20:14
src/client/common/utils/multiStepInput.ts Outdated Show resolved Hide resolved
Copy link

@DonJayamanne DonJayamanne left a comment

Choose a reason for hiding this comment

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

Would like to see the InterpreterStateArgs moved out into corresponding file as its not used anywhere else.

src/client/interpreter/configuration/types.ts Outdated Show resolved Hide resolved
package.nls.json Show resolved Hide resolved
src/client/common/utils/multiStepInput.ts Outdated Show resolved Hide resolved
@karrtikr karrtikr closed this Apr 24, 2020
@karrtikr karrtikr reopened this Apr 24, 2020
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@karrtikr karrtikr merged commit c5a1fa8 into microsoft:master Apr 24, 2020
@karrtikr karrtikr deleted the enterInterpreterPath branch April 24, 2020 19:28
@karthiknadig
Copy link
Member

This PR has passed PR validation https://dev.azure.com/ms/vscode-python/_build/results?buildId=75832&view=results for some reason git is not updating

@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
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.

7 participants