[CCI] Add bluebird replaces for packages/osd-opensearch-archiver #4024
Conversation
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
Nicksqain
requested review from
ananzh,
kavilla,
seanneumann,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
kristenTian,
zhongnansu and
manasvinibs
as code owners
Codecov Report
@@ Coverage Diff @@
## main #4024 +/- ##
=======================================
Coverage 66.44% 66.44%
=======================================
Files 3229 3229
Lines 62067 62067
Branches 9599 9599
=======================================
Hits 41238 41238
Misses 18526 18526
Partials 2303 2303
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -44,7 +44,7 @@ import { | |||
| } from '../lib'; | |||
|
|
|||
| async function isDirectory(path: string): Promise<boolean> { | |||
| const stats: Stats = await fromNode((cb) => stat(path, cb)); | |||
| const stats: Stats = await promisify(stat)(path); | |||
There was a problem hiding this comment.
Should use fs/promises:
import { stat } from 'fs/promises';| const stats: Stats = await promisify(stat)(path); | |
| const stats: Stats = await stat(path); |
There was a problem hiding this comment.
Sorry, I was sticking to description #3662
I'll look into it
There was a problem hiding this comment.
Don't apologize. I should thank you for doing all of this!
| @@ -82,7 +82,7 @@ export async function rebuildAllAction({ | |||
| createWriteStream(tempFile), | |||
| ] as [Readable, ...Writable[]]); | |||
|
|
|||
| await fromNode((cb) => rename(tempFile, childPath, cb)); | |||
| await promisify(rename)(tempFile, childPath); | |||
There was a problem hiding this comment.
Should use fs/promises as well.
|
|
||
| export async function readDirectory(path: string) { | ||
| const allNames = await fromNode<string[]>((cb) => readdir(path, cb)); | ||
| const allNames: string[] = await promisify(readdir)(path); |
There was a problem hiding this comment.
Should use fs/promises as well.
| function delay(ms: number) { | ||
| return new Promise((resolve) => setTimeout(resolve, ms)); | ||
| } | ||
|
|
There was a problem hiding this comment.
We have been defining this delay function way too often. We should move this out into an importable.
There was a problem hiding this comment.
@AMoo-Miki
Thank you for ur review! I was wondering about that, so maybe you can tell me where I can place the delay function?
#3662 (comment)
There was a problem hiding this comment.
packages/osd-utils would be a perfect spot; just throw it into a new file. This way when we get rid of Node 14, we can just use timers/promises and delete this file.
|
convert to draft since requested changes have not been addressed for more than two weeks |
|
@Nicksqain Due to an extended period without updates, we're going to close this issue for now. We appreciate your contribution and understand that priorities and availability can change. Please feel free to reopen this issue when you have the opportunity to revisit it. Thank you for your understanding. |
Description
Issues Resolved
Check List
yarn test:jestyarn test:jest_integrationyarn test:ftr