Make migrations more explicit and generic

[uptrace]

Make migrations more explicit and generic

[Codebreaker101]

Not really, but I think they can be useful at some point (especially context). I am not sure, but since it is a public API I would not change the signature without a good reason.

I agree with the context but I'm trying to figure out any future need for bun.Db. In most, if not all cases, creating migration is done during development phase where one might not have access to database without first spinning up one. One thing I could come up with where database access would be useful is to load templates from database (I'm spit-balling here) and in that case I'm sure a CreateSQLFromDB function would be a more adequate way of doing it. Or maybe generating migration based on diff between last migration and new database state. Again, separate function might be better.

See below how I suggest to extend the API with options

That sounds good.

It used to work this way, but once I broke something (probably changed db schema name) and the app attempted to initialize a new migrations table and re-run all the migrations from scratch in production environment.

That is an interesting problem. Most of my apps are self-initializing and if I would to change table and locksTable at a later time I would have the same problem. I guess the only solution would be to check if the database has any tables before auto-initializing. But that is database specific.

For the rest of the points not mentioned here I will start working on the PRs

[vmihailenco]

Add more code to generated go template like the one bellow since this is a MVP for go migration file.

This was addressed by 98281aa.

[vmihailenco]

@isgj yes, probably it should be the default behavior. Empty migrations should work fine and users can delete down migration if they don't need it.

[vmihailenco]

@isgj should be done in v0.3.4

[vmihailenco]

All of that sounds good 👍 We just need to figure out the details :) I think it will be easier if you split these changes into multiple PRs so we can merge it faster and there is a sense progress being made.

function since they are not used. Is it planned for the future?

Not really, but I think they can be useful at some point (especially context). I am not sure, but since it is a public API I would not change the signature without a good reason. See below how I suggest to extend the API with options.

Make package name configurable for CreateGo migration (and/or item 6.)

What about keeping the existing signature and instead adding an option:

func CreateGo(ctx context.Context, db *bun.DB, name string, opts ...GoOption) error

migrations.CreateGo(ctx, app.DB(), c.Args().Get(0), migrations.WithGoPackage("something"))

This way we can add more options in backwards compatible manner.

Also should not directory string be a global option?

var Migrations = migrate.NewMigrations(migrate.WithDirectory(directory))

Add a check before migrate if the required migration tables exist, if not, initialize. Quick and dirty way is to run Init(...) on every migration. Might be cheaper to execute then checking first. But speed is not an issue here.

It used to work this way, but once I broke something (probably changed db schema name) and the app attempted to initialize a new migrations table and re-run all the migrations from scratch in production environment.

Luckily for me nothing was broken, but since then I prefer to require explicit initialization. But we can add an option like WithAutoInit() if you insist.

[vmihailenco]

Again, separate function might be better.

Let's try it 👍

Part of the problem is that migrate is a library, but it is used as a CLI program so it complicates things. For example, it makes sense to change Status function to return a struct, but then the CLI should somehow print that struct.

I try to address that by providing a starter-kit, but it still can be challenging to use the API correctly....

[Codebreaker101]

@vmihailenco Great!

Then all that remains here is point 6 with maybe point 3:

Add option for adding user template for go and sql migration file creation
Make package name configurable for CreateGo migration

What do you think?

[vmihailenco]

Make package name configurable for CreateGo migration

This looks rather straightforward:

migrator.CreateGo(ctx, name, WithPackageName("main"))

Add option for adding user template for go and sql migration file creation

I am not sure about the template, because then we also need to add template data, funcs and so on. What about just allowing to specify migration content:

migrator.CreateGo(ctx, name, WithMigrationContent("go source code here"))

And WithMigrationContent will ignore WithPackageName option.

[isgj]

Sorry for jumping in and interrupting the discussion. Can there be also an option when creating the sql migration to create also the down migration.?

[Codebreaker101]

@vmihailenco excellent work. I think all of the points are resolved so I am closing this. Thanks!