Skip to content

fix: prefer fs/promises over promisify#7399

Merged
wraithgar merged 1 commit into
latestfrom
lk/more-fs-promises
Apr 24, 2024
Merged

fix: prefer fs/promises over promisify#7399
wraithgar merged 1 commit into
latestfrom
lk/more-fs-promises

Conversation

@lukekarrys

Copy link
Copy Markdown
Contributor

No description provided.

@lukekarrys lukekarrys requested a review from a team as a code owner April 22, 2024 05:33
@wraithgar

Copy link
Copy Markdown
Contributor

This is likely to collide w/ my "remove tables" PR, I'll push it up tomorrow and would like to land that first.

Comment thread lib/commands/doctor.js Outdated
Comment thread lib/commands/edit.js
@lukekarrys lukekarrys force-pushed the lk/more-fs-promises branch 3 times, most recently from d8624ed to 67b9431 Compare April 22, 2024 21:06
@lukekarrys lukekarrys force-pushed the lk/more-fs-promises branch from 67b9431 to c796af3 Compare April 22, 2024 21:06
Comment thread workspaces/arborist/lib/arborist/isolated-reifier.js
Comment thread workspaces/arborist/scripts/benchmark.js
Comment thread workspaces/arborist/scripts/benchmark/reify.js
@wraithgar

Copy link
Copy Markdown
Contributor

Arborist had more than needed individual comments but it seems we aren't consistently using fs or fs/promises to require the sync methods. This is a nitpick for sure but let's just do the same thing everywhere in the off chance we require more methods later.

Also should it be node:fs/promises?

@lukekarrys

Copy link
Copy Markdown
Contributor Author

The sync methods are not available from fs/promises so if we use them from the fs returned from fs/promises it should error somewhere.

@wraithgar

Copy link
Copy Markdown
Contributor

After reviewing again I was wrong, I remembered the sync part but it's also requring from fs

@lukekarrys lukekarrys force-pushed the lk/more-fs-promises branch from ae83cb3 to c796af3 Compare April 23, 2024 17:12
@wraithgar wraithgar merged commit 78447d7 into latest Apr 24, 2024
@wraithgar wraithgar deleted the lk/more-fs-promises branch April 24, 2024 16:26
@github-actions github-actions Bot mentioned this pull request Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants