feat: add retrievetargetdir to source:retrieve #624
Conversation
@W-10735446@
…ale/flag-retrieve-target-dir
…ale/flag-retrieve-target-dir
| let files: string[] = []; | ||
| const srcStat = await fs.promises.stat(src); | ||
| if (srcStat.isDirectory()) { | ||
| const contents = await fs.promises.readdir(src); |
There was a problem hiding this comment.
you can save a lot of fs ops by using the withFileTypes option on readdir
https://nodejs.org/api/fs.html#fspromisesreaddirpath-options
which gives you the full dirEnt info https://nodejs.org/api/fs.html#class-fsdirent
src/promiseQueue.ts
Outdated
| const nextResults = (await Promise.all(next.map(producer))) | ||
| .reduce<T[]>((acc, val) => { | ||
| if (Array.isArray(val)) { | ||
| acc.push(...val); | ||
| } else { | ||
| acc.push(val); | ||
| } | ||
| return acc; | ||
| }, []) | ||
| .filter((val) => val !== undefined); |
There was a problem hiding this comment.
const nextResults = (await Promise.all(ensureArray(next.map(producer)))).flatMap((val) => val);
| sourceQueue: T[], | ||
| producer: (T) => Promise<T | T[]>, | ||
| concurrency: number, | ||
| queueResults = false |
There was a problem hiding this comment.
I'd omit this since it isn't used. It's not clear what it might do and at-a-glance invites an infinite loop
There was a problem hiding this comment.
@mshanemc it is used in mv function. Yes it can and yes I have :). Not being able to queue results defeats the ability to accomplish the dir traversal without recursion.
| @@ -0,0 +1,43 @@ | |||
| /* | |||
There was a problem hiding this comment.
overall comment...there's some tools that do this kind of thing if we find a lot more use cases for it.
https://github.com/supercharge/promise-pool is fairly small, zero-dep. The error handling and abort looks nice.
| return false; | ||
| }); | ||
| if (overlapsProject) { | ||
| return; |
There was a problem hiding this comment.
this feels like a good place for a warning. Another option would be to make this "overlaps" validation a standalone function that could be used for your new flag's parse so that it doesn't go through all that retrieve work to fail this late in the game.
As is, it looks like the user would specify a targetDir and then if it overlapped, the retrieve would go to their default dir anyway without a warning?
There was a problem hiding this comment.
@mshanemc I think I am going to change direction on overlapping targets, fail the command if the target overlaps a package dir. There are other commands and variants of source:retrieve to get source into a package dir.
| return; | ||
| } | ||
| let overlapsProject = true; | ||
| const resolvedTargetDir = resolve(this.flags.retrievetargetdir as string); |
There was a problem hiding this comment.
and what if I want to "peek" at the results, outside my normal project. So I say, --retrievetargetdir peek/main/default
what's going to happen? .replace(join('main', 'default') would eliminate the redundancy?
There was a problem hiding this comment.
I'll give that a try, but my suspect that it will only remove one main/default
There was a problem hiding this comment.
As predicted, the results land in peek/main/default
Co-authored-by: Shane McLaughlin <[email protected]>
Co-authored-by: Shane McLaughlin <[email protected]>
Co-authored-by: Juliet Shackell <[email protected]>
|
QA notes: ✅ retrieve classes to new dir ❌ retrieve to new child folder of pkgDir ✅ retrieve to same dir twice |
|
✅ force-app hits error couldn't find any additional problems. 🎉 |
What does this PR do?
add retrievetargetdir to source:retrieve
What issues does this PR fix or reference?
@W-10735446@