-
Notifications
You must be signed in to change notification settings - Fork 518
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
feat: goose validate
command
#449
Conversation
So this is a sanity check for the SQL migrations, right? Do the numbers represent count of SQL statements? Go migrations would be nice to support too, I guess we could print Thinking of better names: |
Yep, intended to be a command users can run to sanity-check their migration files. I noticed some users only find out about malformed migration files (especially down migrations) when it's too late. Here's a real-world example, where migration 35 is missing a semicolon. And running this proposed command would catch this error:
A lot of projects don't have a robust set of tests like we have, i.e., up, down, up then drop schema and compare it to the checked-in schema. Great idea to include the .go migrations! |
wrote a simple parser for
|
Still can't think of a good name for this command, cc @VojtechVitek what do you think? Suggestions welcome... I think
Other examples
https://developer.hashicorp.com/terraform/cli/commands/validate |
validate is a pretty good name :) |
"text/template" | ||
|
||
"github.com/pressly/goose/v3" | ||
"github.com/pressly/goose/v3/internal/cfg" | ||
"github.com/pressly/goose/v3/internal/migrationstats" |
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.
It's possible folks might want this functionality exposed in the goose package, if so, we can move this to the top level or create a pkg
with helpful utilities unrelated to migration logic per se.
Pretty happy with how this turned out, been finding this command awfully useful. Some nice-to-have for the future:
|
Fixes #448