Use the "withFileTypes" options with fs.readdir().#14994
Merged
ericsnowcurrently merged 36 commits intomicrosoft:mainfrom Jan 11, 2021
Merged
Conversation
kimadeline
reviewed
Dec 17, 2020
kimadeline
left a comment
There was a problem hiding this comment.
Do we have any tests that need to be updated as part of this PR?
karrtikr
approved these changes
Dec 17, 2020
karrtikr
reviewed
Dec 17, 2020
karrtikr
reviewed
Dec 17, 2020
| export function listDir(dirname: string): Promise<string[]> { | ||
| return fsapi.readdir(dirname); | ||
| export function listDir(dirname: string): Promise<fs.Dirent[]> { | ||
| return fs.promises.readdir(dirname, { withFileTypes: true }); |
There was a problem hiding this comment.
Just wanted to point out that the assumption is that passing this option does not make things any more expensive than it previous was.
Author
There was a problem hiding this comment.
It makes it cheaper, in fact. 😄
kimadeline
approved these changes
Dec 17, 2020
kimadeline
left a comment
There was a problem hiding this comment.
My comments above are not blocking.
Author
|
There aren't any tests already for |
kimadeline
pushed a commit
to kimadeline/vscode-python
that referenced
this pull request
Dec 21, 2020
ericsnowcurrently
pushed a commit
that referenced
this pull request
Jan 5, 2021
2483ede to
27ac913
Compare
karrtikr
reviewed
Jan 6, 2021
src/test/pythonEnvironments/common/commonUtils.functional.test.ts
Outdated
Show resolved
Hide resolved
karrtikr
reviewed
Jan 6, 2021
| matched = cfg.filterFile(filename); | ||
| } catch (err) { | ||
| if (cfg.ignoreErrors) { | ||
| logError(`filterFile() failed for "${filename}" (${err})`); |
There was a problem hiding this comment.
I was expecting us to continue here instead of return.
There was a problem hiding this comment.
Also, I expect to ignore errors and continue by default.
karrtikr
suggested changes
Jan 6, 2021
karrtikr
approved these changes
Jan 6, 2021
Codecov Report
@@ Coverage Diff @@
## main #14994 +/- ##
=======================================
Coverage 65% 65%
=======================================
Files 557 560 +3
Lines 26215 26358 +143
Branches 3727 3745 +18
=======================================
+ Hits 17060 17160 +100
- Misses 8459 8501 +42
- Partials 696 697 +1
|
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For #14983.
[ ] Has a news entry file (remember to thank yourself!).[ ] Has telemetry for enhancements.[ ] Unit tests & system/integration tests are added/updated.[ ] Test plan is updated as appropriate.[ ]package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).[ ] The wiki is updated with any design decisions/details.