-
Notifications
You must be signed in to change notification settings - Fork 1
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
Move from github.com/mattes/migrate => github.com/golang-migrate/migrate repo #257
Conversation
Thanks for this! Looks like some test output checking needs to be updated. Also, what should folks who still have github.com/mattes/migrate in heroku.additionalTools expect to happen? |
Was not aware of the test cases. Will take care of it.
That's a valid question. I am not sure. So far I would assume that no-one uses that feature as it actually does not work. So we might not have to worry about that case. But likely only you know the numbers for that. |
We do use it internally. Re it not working based on filename, maybe there is confusion between the filename key here (which I think heroku-buildpack-go/files.json Lines 334 to 337 in 8f0649c
Perhaps the buildpack should support either being in heroku.additionalTools, preferring github.com/golang-migrate/migrate, and printing a warning if only github.com/mattes/migrate is used? |
Should add too there are tests that specifically verify having github.com/mattes/migrate in heroku.additionalTools works, such as: Lines 30 to 46 in 8f0649c
and: Lines 159 to 176 in 8f0649c
|
This won't work as it breaks existing users who could push afterwards and not get mattes migrate as they expected. |
See #264 for how I would like to do it so as to not break existing users and allow them some time to migrate. |
@freeformz makes sense and thanks for taking over this issue! 🙇 |
Fixes #256.
Also fixes the filename download URL:
migrate.linux-amd64.tar.gz
notmigrate-linux-amd64.tar.gz
.