fix migration and add smoke test for migrations #438

2024-02-11T23:26:24+08:00

adrienpoly commented

2024-02-11 23:26:24 +08:00

(Migrated from github.com)

somehow money-rail gem was removed in a previous commit
as a consequence I wasn't able to run the migration to start the project

~~this PR adds it back~~ and adds a smoke database reset test to ensure a new developper can always run migrations to start fresh on this project

somehow money-rail gem was removed in a previous commit as a consequence I wasn't able to run the migration to start the project ~~this PR adds it back~~ and adds a smoke database reset test to ensure a new developper can always run migrations to start fresh on this project

JoshAntBrown commented

2024-02-12 00:58:14 +08:00

(Migrated from github.com)

I think it may be better to remove the dependency from the migration than bring back dead dependencies?

adrienpoly commented

2024-02-12 01:01:19 +08:00

(Migrated from github.com)

Sure I don’t have a detailed history of the project looking through the PRs I was under the impression the gem was removed by mistake. If that’s not the case then for sure I would prefer to fix the migration

👍 1

JoshAntBrown commented

2024-02-12 01:08:36 +08:00

(Migrated from github.com)

Ah yes of course, checking git blame looks like it was removed in this PR: https://github.com/maybe-finance/maybe/pull/425. There's some discussion about this issue there too.

👍 1

adrienpoly commented

2024-02-12 01:13:18 +08:00

(Migrated from github.com)

Great will update the PR to fix this migration without the gem them

👍 1

adrienpoly commented

2024-02-12 01:34:46 +08:00

(Migrated from github.com)

Pr updated accordingly. Thanks for pointing me in the good direction

👍 1 🎉 1

Sign in to join this conversation.