Simplify source code while visiting few cli/ sources
#125
Conversation
|
This change attempts to separate the Xcframework linking logic from the directory iteration by extracting the "glob-like" function out. This change also opens the door for easier migration to a proper glob() function.
| withFileTypes: true, | ||
| }) | ||
| // Following extracted function mimics `glob("*/*.framework/")` | ||
| function globFrameworkDirs<T>( |
There was a problem hiding this comment.
Nit: this can be moved to path-utils
There was a problem hiding this comment.
Since this also applies a side-effect, it feels to me that it's doing a bit more than simply globbing?
Perhaps
| function globFrameworkDirs<T>( | |
| function globAndPrepareFrameworks<T>( |
mani3xis
changed the title
cli/ sourcescli/ sources
| // Expect the library in the new location | ||
| assert(fs.existsSync(newLibraryPath)); | ||
| // Update the binary | ||
| await spawn( |
There was a problem hiding this comment.
nit: imho this should be extracted to a separate function
| path.join(dependency.path, modulePath) | ||
| ); | ||
| } | ||
| const absoluteModulePaths = Object.values(dependenciesByName).flatMap( |
| } else { | ||
| throw error; | ||
| } | ||
| absoluteModulePaths.map(async (originalPath) => { |
| // Following extracted function mimics `glob("*/*.framework/")` | ||
| function globFrameworkDirs<T>( | ||
| startPath: string, | ||
| fn: (parentPath: string, name: string) => Promise<T> |
There was a problem hiding this comment.
Could we pick a more semantic name for this argument? It's like an action performed on every triplet + framework.
I'm in the process of fixing some errors that I'm getting after integrating major changes from
main, and while looking for the root-cause of those issues, I decided to do some minor code cleanups and simplifications while visiting some of the files.This PR will leave the DRAFT status as I fix all the issues.