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

Added support for a custom migration table name #207

Merged
merged 8 commits into from
Jan 6, 2022
Merged

Added support for a custom migration table name #207

merged 8 commits into from
Jan 6, 2022

Conversation

TobiasDeBruijn
Copy link
Contributor

@TobiasDeBruijn TobiasDeBruijn commented Jan 5, 2022

This PR adds support for a custom name for the migration table (by default refinery_schema_history).

Motivation

I've seen quite some issues requesting this is one way or another:

Approach

The PR adds a new field to the Runner struct, migration_table_name and an associated function set_migration_table_name. The value of migration_table_name is passed around to the places it needs to be as a &str. In refinery_core::traits, there are 3 const &str's, which now include a 'variable' %MIGRATION_TABLE_NAME% which must be replaced before being used, this is done everywhere correctly.

Backwards compatibility

This PR does not break backwards compatibility, mostly. For most binaries, this PR does not break backwards compatibility. However, the traits Migrate and AsyncMigrate have had changes to their functions, the only change to each function is the addition of a new parameter, migration_table_name. I doubt anyone uses these traits outside of the refinery crate, but it is a breaking change.

Modification to tests

The tests have been modified to conform with the updated function signatures, i.e. the migration_table_name parameter

Thanks.

@TobiasDeBruijn
Copy link
Contributor Author

Implementation is done then :)

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nice work thanks! <3 Only some minor situations left

refinery_core/src/drivers/tiberius.rs Outdated Show resolved Hide resolved
refinery_core/src/runner.rs Outdated Show resolved Hide resolved
refinery_core/src/traits/sync.rs Outdated Show resolved Hide resolved
refinery_core/src/traits/async.rs Outdated Show resolved Hide resolved
- The iter call isn't needed indeed, and can be replaced by adding an &
- Added const DEFAULT_MIGRATION_TABLE_NAME
- Removed TODO from tiberius driver
@jxs jxs merged commit 99e1f87 into rust-db:main Jan 6, 2022
@jxs
Copy link
Member

jxs commented Jan 6, 2022

Awesome, thanks!
I think we can release a minor as this isn't breaking for refinery. refinery_core should not be used alone and it's explicitly written on its description so I think it's safe

@TobiasDeBruijn
Copy link
Contributor Author

TobiasDeBruijn commented Jan 6, 2022

Might be best to wait a little with releasing. I'd also like to implement the custom schema for the schema table ( see my comment in #205), that could go in the same minor release

Edit: err, disregard. Seems I completely misunderstood the meaning of schema 😅

@jxs jxs changed the title Added support for a custum migration table name Added support for a custom migration table name Jan 9, 2022
@jxs jxs mentioned this pull request Dec 10, 2022
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.

2 participants