Skip to content
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

Include missing migrations #140

Closed
wants to merge 89 commits into from
Closed

Include missing migrations #140

wants to merge 89 commits into from

Conversation

elijahcarrel
Copy link

@elijahcarrel elijahcarrel commented Jan 25, 2019

This is based off of @Stepych's P.R. which adds the option to include missing migrations when goosing up. I replaced references to mc2soft with pressly so that it can feasibly be merged and made the inclusion of missing migrations optional. I also did some general refactoring.

I hope that the change is relatively straightforward to review, but in essence this will add a flag -include-missing to the up and up-to commands. When the flag is passed, goose will also apply migrations that may have been skipped (which can happen if they were added after the user had already goose upped past them). Additionally, it adds a flag -missing-only to the status command, which will show only migrations that were skipped.

Stepych and others added 30 commits October 3, 2017 14:17
…ate current DB. Accepted by up and up-by-one commands.
Copy link
Collaborator

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elijahcarrel Thanks for your contribution. However, I've recently closed the original issue #73 (PR #72), since I believe we shouldn't apply any missing migrations.

We explicitly prefer to error out if there's any gap in the migrations. If goose skips those migrations right now, it's probably a bug and we should fix that and make it an error.

We don't want to apply any migrations that are out of the sequence number order.

There shouldn't be any "skipped" migration if you version your migrations properly. See https://github.com/pressly/goose#hybrid-versioning for further explanation.

Closing this PR.

Feel free to submit a new issue with detailed description of the problem, if you believe this is wrong decision - even after reading about Hybrid versioning. Thanks!

btw: FYI, 89 commits is a lot. I would have asked you to squash this PR.

Thanks again. Cheers

func UpTo(db *sql.DB, dir string, version int64) error {
migrations, err := CollectMigrations(dir, minVersion, version)
// Up performs all types of migrations upwards, depending on the params.
func Up(db *sql.DB, dir string, includeMissing bool, onlyOne bool, endVersion *int64) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw: There are API breaking changes; we would have to consider this for goose v3, but not earlier.

@elijahcarrel
Copy link
Author

Out of order migrations have been added in a backwards-compatible way in #280

@mfridman
Copy link
Collaborator

There's a bit more detail here: https://pressly.github.io/goose/blog/2021/out-of-order-migrations/

Also, it took a while @elijahcarrel 😅 thanks for trying to pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet