-
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
[109] Fix: Cannot run a project if it does not have any DB migration #114
base: develop
Are you sure you want to change the base?
Conversation
log.Fatalf("Failed to migrate database: %v", err) | ||
} | ||
|
||
log.Println("Migrated database successfully.") |
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.
log.Println("Migrated database successfully.") | |
log.Println("Database migrated successfully.") |
😇
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.
Thanks, changed in 4611bd3
{{cookiecutter.app_name}}/Makefile
Outdated
ifneq ("$(wildcard $(database/migrations/.keep))","") | ||
goose -dir database/migrations -table "migration_versions" postgres "$(DATABASE_URL)" up | ||
else | ||
$(info NO migration files to run) |
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.
ifneq ("$(wildcard $(database/migrations/.keep))","") | |
goose -dir database/migrations -table "migration_versions" postgres "$(DATABASE_URL)" up | |
else | |
$(info NO migration files to run) | |
ifeq ("$(wildcard $(database/migrations/.keep))","") | |
$(info NO migration files to run) | |
else | |
goose -dir database/migrations -table "migration_versions" postgres "$(DATABASE_URL)" up |
Not sure if that work, but can we use an ifeq
so we don't need to process the negative form (if NOT X do A else B vs
if X do B else A`)
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 works so changed in 1183eb4 🙌🏼
Resolve #109
What happened 👀
Insight 📝
N/A
Proof Of Work 📹
After generating the app, install the dependencies and run
make dev
Before: the app crashed because there was NO migration file to run
Screen.Recording.2566-09-29.at.17.47.12.mov
After: the app is runnable with the message tells there's no migration file
Screen.Recording.2566-09-29.at.17.38.18.mov
The health API works! 🎉