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

Rename CheckPending to GetVersions #764

Merged
merged 2 commits into from
May 3, 2024
Merged

Conversation

mfridman
Copy link
Collaborator

@mfridman mfridman commented May 3, 2024

Picking up from #762 (comment)

I don't want to give the impression this method is suitable for checking "pending" migrations because it's a bit nuanced. I.e., what if a missing (out-of-order) migration has snuck in, it'd be silently ignored if we just compare the max db version and target filesystem version.

This is what HasPending is intended to do, handle all the edge cases around missing migrations (which can be enabled or disabled). For a better understanding of all the possible cases see:

https://github.com/pressly/goose/blob/8c8def43/provider.go#L530-L544 and https://github.com/pressly/goose/blob/8c8def43/internal/gooseutil/resolve_test.go#L73-L83

Once you enumerate the edge cases it becomes clear that internally we cannot compare latest versions, e.g., what happens when running goose up-to 4 and the current db version is 6, where the "missing" migration is 5? I'll write up a reference for "What is a missing migration?"

TL;DR

So, rename CheckPending to GetVersions which does what it says on the tin: get the (max) version from the database and filesystem, which are termed current and target, respectively.

@mfridman mfridman merged commit 992628d into master May 3, 2024
5 checks passed
@mfridman mfridman deleted the mf/rename-checking-pending branch May 3, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant