Skip to content

simplified FileFinder utility#148

Closed
FND wants to merge 1 commit into
mainfrom
mdrnz-files
Closed

simplified FileFinder utility#148
FND wants to merge 1 commit into
mainfrom
mdrnz-files

Conversation

@FND

@FND FND commented Mar 6, 2025

Copy link
Copy Markdown
Contributor

I'm not sure it's wise to touch any of this without any tangible benefit - not least because I'm not sure test coverage is sufficient to provide confidence.

In my defense, it started as an effort to replace the obsolete flatten utility...

mostly `async`/`await` and revisiting module-level utility functions
@FND
FND requested a review from moonglum March 6, 2025 13:19
Comment thread lib/util/files/finder.js
Comment on lines +5 to 6
constructor(directory, { skipDotfiles, filter } = {}) {
this.directory = directory;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
constructor(directory, { skipDotfiles, filter } = {}) {
this.directory = directory;
constructor(root, { skipDotfiles, filter } = {}) {
this._root = root;

It doesn't look like the directory property is used by anyone; I've checked faucet-static and faucet-images.

Comment thread lib/util/files/finder.js
match(filepaths) {
return filesWithinDirectory(this.directory, filepaths).
then(filepaths => filepaths.filter(this.filter));
async match(filepaths) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

async isn't actually required by the implementation nor by downstream (i.e. faucet-static and faucet-images), but current tests demand it...

... except if the filter option is async, so I guess it's best to leave this as is, if only because it allows for more flexibility.

@FND FND mentioned this pull request Mar 6, 2025
@moonglum

Copy link
Copy Markdown
Member

Thanks for your work on this, it has been completely merged into faucet-pipeline/faucet-pipeline-static#71

@moonglum moonglum closed this Mar 15, 2025
@FND
FND deleted the mdrnz-files branch March 16, 2025 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants