-
Notifications
You must be signed in to change notification settings - Fork 952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing issue with firebase init genkit on Windows #8011
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8011 +/- ##
==========================================
- Coverage 51.19% 51.18% -0.01%
==========================================
Files 423 423
Lines 29544 29550 +6
Branches 6031 6031
==========================================
+ Hits 15125 15126 +1
- Misses 13059 13064 +5
Partials 1360 1360 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some nits - thanks for throwing in some extra code cleanup too!
"For a possible workaround run\n npm view genkit version\n" + | ||
"and then set an environment variable:\n" + | ||
" export GENKIT_DEV_VERSION=<output from previous command>\n" + | ||
"and try the firebase init command again", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"and try the firebase init command again", | |
"and run `firebase init` again", |
@@ -482,7 +483,7 @@ export async function writeSDK( | |||
// Build it | |||
logLabeledBullet("extensions", `running 'npm --prefix ${shortDirPath} run build'`); | |||
try { | |||
execFileSync("npm", ["--prefix", dirPath, "run", "build"]); | |||
await spawnWithOutput("npm", ["--prefix", dirPath, "run", "build"]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Since we don't use the output, should we be using wrapSpawn in this file instead?
Fixing issue with firebase init genkit on Windows.