-
Notifications
You must be signed in to change notification settings - Fork 105
Support parsing multiple SQL statements in sql2pgroll.Convert
#690
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
Conversation
b92cd27
to
235b2f7
Compare
235b2f7
to
25d7008
Compare
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.
Left some comments.
More generally, could the signature of Convert
remain as returning a slice of operations? The process of putting them into a Migration
and generating a name for the migration seems like functionality that should be in whatever is calling Convert
, not in Convert
itself.
25d7008
to
54bed01
Compare
@andrew-farries I have reworked the pull request to keep the signature of |
sql2pgroll.Convert
to return Migration
and parse multiple statementssql2pgroll.Convert
54bed01
to
0e3748f
Compare
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.
Looks good 👍
pkg/sql2pgroll/convert_test.go
Outdated
expectedErr bool | ||
}{ | ||
"empty SQL statement": { | ||
sql: "", |
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.
could fill in the expectedOps
and expectedErr
fields here too.
This PR introduces support for translating multiple SQL statements to pgroll operations in
sql2pgroll.Convert
.This PR required by the upcoming improvements to support reading complete SQL migration
files produced by ORMs.