-
Notifications
You must be signed in to change notification settings - Fork 950
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
sql:migrate --force
shouldn't prompt
#7208
Conversation
sql:migrate --force
shouldn't prompt
): Promise<"none" | "safe" | "all"> { | ||
err: IncompatibleSqlSchemaError | undefined, | ||
validateOnly: boolean, | ||
): Promise<"none" | "all"> { |
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.
Since there's only two options now, should this return Promise<false | true>
? Also does maybeExecuteSchemaMigration
better describe what this method does?
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.
This method doesn't execute schema migration. It determines whether to execute it later.
Well, keeping it as string merely because we will have a "compatible" mode soon. At least 3 value enum is needed here.
Thanks! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7208 +/- ##
==========================================
- Coverage 54.10% 54.06% -0.05%
==========================================
Files 387 386 -1
Lines 26094 26108 +14
Branches 5358 5361 +3
==========================================
- Hits 14119 14115 -4
- Misses 10715 10733 +18
Partials 1260 1260 ☔ View full report in Codecov by Sentry. |
Description
firebase dataconnect:sql:migrate --force
should never prompt.firebase dataconnect:sql:migrate
should prompt for schema migrations and then abort if there is invalid connectors. This way developers can see the SQL before taking the suggesting and run it with--force
.Scenarios Tested
Sample Commands
Without invalid connectors
firebase dataconnect:sql:migrate
firebase dataconnect:sql:migrate --force
With invalid connectors
firebase dataconnect:sql:migrate
firebase dataconnect:sql:migrate --force