feat: find files func in prep for auto detection #886
Merged
Conversation
This file contains 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
ghost
requested review from
gitphill
force-pushed
the
feat/helper-to-find-files
branch
from
85aa30b
to
8456605
Compare
orsagie
reviewed
Nov 28, 2019
src/lib/find-files.ts
Outdated
| const stats = await getStats(searchPath); | ||
| if (stats.isDirectory()) { | ||
| const paths = await readDirectory(searchPath); | ||
| for (const subPath of paths) { |
There was a problem hiding this comment.
Do we want to do this asynchronously for found paths?
There was a problem hiding this comment.
Not sure I understand, could you elaborate?
There was a problem hiding this comment.
I think she is saying you can do Promise.all() instead of for loop (maybe with a pMap to reduce concurrency)
lili2311
reviewed
Nov 28, 2019
lili2311
reviewed
Nov 28, 2019
lili2311
reviewed
Nov 28, 2019
src/lib/find-files.ts
Outdated
| const found: string[] = []; | ||
| try { | ||
| const stats = await getStats(searchPath); | ||
| if (stats.isDirectory()) { |
There was a problem hiding this comment.
this whole function is quite hard to read with so many if else, maybe split it up and return early where possible?
There was a problem hiding this comment.
yea I'll take a look, it is a bit intricate😃
lili2311
reviewed
Nov 28, 2019
src/lib/find-files.ts
Outdated
| ): Promise<string[]> { | ||
| const found: string[] = []; | ||
| try { | ||
| const stats = await getStats(searchPath); |
There was a problem hiding this comment.
maybe filePathStats to be more explicit?
gitphill
force-pushed
the
feat/helper-to-find-files
branch
2 times, most recently
from
386417d
to
e87c2ac
Compare
Added find-files module with find function to help search for files. Added test for find and test fixtures. Ignore node_modules in sub directories or as given path. Co-authored-by: Lili Kastilio <lili@lilianakastilio.co.uk>
gitphill
force-pushed
the
feat/helper-to-find-files
branch
from
e87c2ac
to
8ef2bd9
Compare
lili2311
approved these changes
Dec 6, 2019
|
🎉 This PR is included in version 1.258.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Sep 15, 2021
This was referenced Sep 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do?
Added find-files module with find function to help search for files.
Added test for find and test fixtures.
How should this be manually tested?
Just helper function at the moment, separate PR to integrate with CLI commands and auto detection.
What are the relevant tickets?
BST-1072