Skip to content

Remove workspaceFolders from app because it turned out not to be useful#2497

Merged
robertbrignull merged 4 commits intomainfrom
robertbrignull/cleanup_workspace_folders
Jun 12, 2023
Merged

Remove workspaceFolders from app because it turned out not to be useful#2497
robertbrignull merged 4 commits intomainfrom
robertbrignull/cleanup_workspace_folders

Conversation

@robertbrignull
Copy link
Copy Markdown
Contributor

This PR basically just reverts #2431 and #2437. Since we've decided to use integration tests for the queries panel and not try to push for unit tests it's meant that neither of those changes were particularly useful, or are possibly detrimental.

  • workspaceFolders was completely unused, because we have the getOnDiskWorkspaceFoldersObjects helper and it made much more sense to call that.
  • onDidChangeWorkspaceFoldersmade little difference, since mocking the VS Code version was just as easy.
  • createEventEmitter wasn't terribly helpful because mocking the VS Code version wasn't hard. It was actually possibly harmful because the default implementation in createMockApp silently does nothing, which tripped me up when writing tests and my listeners weren't firing. You haven't to know to pass a working emitter constructor when making the mock app, but there's no error that will point you in that direction.

This PR will conflict with #2490, but it's not a big problem. I still want to make these changes and I'll fix any conflicts on that PR.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@robertbrignull robertbrignull requested a review from a team June 9, 2023 09:42
@robertbrignull robertbrignull marked this pull request as ready for review June 9, 2023 09:43
@robertbrignull robertbrignull requested a review from a team as a code owner June 9, 2023 09:43
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning that up!

@robertbrignull robertbrignull merged commit 6dbbd22 into main Jun 12, 2023
@robertbrignull robertbrignull deleted the robertbrignull/cleanup_workspace_folders branch June 12, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants