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

Allow users to specify the migrations "tableName" parameter via the CLI. #3214

Merged
merged 2 commits into from May 22, 2019

Conversation

Projects
None yet
2 participants
@zbmott
Copy link
Contributor

commented May 21, 2019

I am using Knex to migrate several projects from MongoDB to PostgreSQL. Since these projects already have a means of storing connection configuration, the knexfile is redundant. Instead, I can read my existing configuration and generate the appropriate Knex migration command dynamically.

However, because my projects share the same PostgreSQL database, I need to namespace their migration histories so that they don't conflict. In order to do that, I need to be able to specify the tableName configuration option via the command line, which this PR introduces support for.

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented May 21, 2019

Thank you for your contribution! Could you please add test for this case in https://github.com/tgriesser/knex/tree/master/test/cli as well as create a documentation PR in https://github.com/knex/documentation?

@zbmott

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

I notice that the project doesn't have any unit tests in place for mkConfigObj. Since JS isn't my first language, can I ask what approach you'd take to testing this new functionality?

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented May 21, 2019

See https://github.com/tgriesser/knex/tree/master/test/cli
You can make an actual CLI call and then use knex instance to query db to see if correct table exists in db. Using sqlite for testing should be fine.

@zbmott

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

Added a test for --migration-table-name CLI option. Opened PR for documentation update: knex/documentation#195

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

Awesome, thank you!

@kibertoad kibertoad merged commit f4593fb into tgriesser:master May 22, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.002%) to 88.652%
Details
@zbmott

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

Thank you for your help preparing this feature for acceptance. Can I ask what you release schedule looks like? As I said in my initial comment, I built this feature out because it's something I need for a project I'm working on. I'd like to upgrade my version of knex and start using this feature as soon as possible.

@zbmott zbmott deleted the zbmott:cli-migration-table-name-option branch May 22, 2019

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

I've just released 0.17.0-next5. Enjoy!

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

@zbmott Please let me know if something doesn't work as expected :)

@zbmott

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.