Drop `workspaceService.hasWorkspaceFolders` by FATESAIKOU · Pull Request #14100 · microsoft/vscode-python · GitHub
Skip to content

Drop workspaceService.hasWorkspaceFolders#14100

Closed
FATESAIKOU wants to merge 1 commit into
microsoft:mainfrom
FATESAIKOU:main
Closed

Drop workspaceService.hasWorkspaceFolders#14100
FATESAIKOU wants to merge 1 commit into
microsoft:mainfrom
FATESAIKOU:main

Conversation

@FATESAIKOU

@FATESAIKOU FATESAIKOU commented Sep 25, 2020 •
edited
Loading

Copy link
Copy Markdown

To stop ignoring warnings from the usage below:
workspaceService.hasWorkspaceFolders

We replaced all of these usage with:
(this.workspaceService.workspaceFolders || []).length > 0

For #6237

  • 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.

@ghost

ghost commented Sep 25, 2020 •
edited by ghost
Loading

Copy link
Copy Markdown

@codecov-commenter

codecov-commenter commented Sep 25, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #14100 into main will decrease coverage by 0.07%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #14100      +/-   ##
==========================================
- Coverage   59.77%   59.69%   -0.08%     
==========================================
  Files         702      702              
  Lines       38868    38868              
  Branches     5618     5620       +2     
==========================================
- Hits        23232    23201      -31     
- Misses      14426    14447      +21     
- Partials     1210     1220      +10
Impacted Files Coverage Δ
src/client/activation/activationService.ts 89.04% <0.00%> (-0.69%) ⬇️
...on/diagnostics/checks/invalidLaunchJsonDebugger.ts 90.58% <0.00%> (-3.53%) ⬇️
...datascience/interactive-common/notebookProvider.ts 50.58% <0.00%> (ø)
src/client/datascience/jupyter/jupyterExporter.ts 15.31% <0.00%> (ø)
src/client/datascience/jupyter/jupyterImporter.ts 17.33% <0.00%> (ø)
...nt/datascience/kernel-launcher/kernelDaemonPool.ts 87.96% <0.00%> (ø)
src/client/interpreter/virtualEnvs/index.ts 74.35% <0.00%> (-1.29%) ⬇️
...nments/discovery/locators/services/condaService.ts 84.04% <0.00%> (ø)
src/client/testing/common/debugLauncher.ts 90.56% <0.00%> (-1.89%) ⬇️
...nt/testing/common/services/configSettingService.ts 88.88% <0.00%> (-1.59%) ⬇️
... 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 66a4cb1...f3ed75e. Read the comment docs.

@FATESAIKOU
Drop workspaceService.hasWorkspaceFolders
f3ed75e 
To stop ignoring warnings from the usage below:
    `workspaceService.hasWorkspaceFolders`

We replaced all of these usage with:
    `(this.workspaceService.workspaceFolders || []).length > 0`
@sonarqubecloud

sonarqubecloud Bot commented Sep 25, 2020

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@FATESAIKOU FATESAIKOU mentioned this pull request Sep 26, 2020
Closed
karthiknadig
karthiknadig requested changes Nov 10, 2020
View reviewed changes

@karthiknadig karthiknadig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@FATESAIKOU Can you change this so that this.workspaceService.workspaceFolders set to [] by default. That way we can avoid doing has check.

@kimadeline kimadeline removed their assignment Nov 16, 2020
@kimadeline

kimadeline commented Nov 16, 2020

Copy link
Copy Markdown

Hi @FATESAIKOU, could you update your PR with the latest code in main? This should help with the merge conflicts.

Thanks!

@kimadeline kimadeline self-assigned this Nov 16, 2020
@kimadeline

kimadeline commented Feb 8, 2021

Copy link
Copy Markdown

@kimadeline kimadeline closed this Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@karthiknadig karthiknadig karthiknadig requested changes
@kimadeline kimadeline Awaiting requested review from kimadeline

Assignees

@kimadeline kimadeline

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@FATESAIKOU @codecov-commenter @kimadeline @karthiknadig