Skip to content

Fix(#656) Ensure scoped migration created in subdirectory #655

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 12, 2020
Merged

Fix(#656) Ensure scoped migration created in subdirectory #655

merged 2 commits into from
Jan 12, 2020

Conversation

AnsonT
Copy link
Contributor

@AnsonT AnsonT commented Nov 9, 2019 •
edited
Loading

Fix for bug: #656

Current if you call
db-migrate create someScope/aMigration --sql-file, the following is created:

  • ./migrations/someScope/xxx-someScope.js
  • ./migrations/sqls/xxx-someScope-up.sql
  • ./migrations/sqls/xxx-someScope-down.sql

Instead, I expect:

  • ./migrations/someScope/xxx-aMigration.js
  • ./migrations/someScope/sqls/xxx-aMigration-up.sql
  • ./migrations/someScope/sqls/xxx-aMigration-down.sql

@commitlint-wzrdtales
Copy link

There were the following issues with this Pull Request

  • Commit: f9c7ef6
    • ✖ message may not be empty
      , - ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.

🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@AnsonT AnsonT force-pushed the fix-create-migration-scope-path branch from f9c7ef6 to 4784ddc Compare November 9, 2019 00:29
@commitlint-wzrdtales
Copy link

There were the following issues with this Pull Request

  • Commit: 4784ddc
    • ✖ message may not be empty
      , - ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.

🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@AnsonT AnsonT force-pushed the fix-create-migration-scope-path branch from 4784ddc to 6df87cd Compare November 9, 2019 00:33
Fix: Ensure scoped migration created in subdirectory
cd128e2 
db-migrate create someScope/aMigration --sql-file, the following is created:

./migrations/someScope/xxx-someScope.js
./migrations/sqls/xxx-someScope-up.sql
./migrations/sqls/xxx-someScope-down.sql
Instead, I expect:

./migrations/someScope/xxx-aMigration.js
./migrations/someScope/sqls/xxx-aMigration-up.sql
./migrations/someScope/sqls/xxx-aMigration-down.sql

Signed-off-by: Anson Tsao <[email protected]>
@AnsonT AnsonT force-pushed the fix-create-migration-scope-path branch from 6df87cd to cd128e2 Compare November 9, 2019 00:35
@AnsonT AnsonT changed the title Ensure scoped migration created in subdirectory Fix(#656) Ensure scoped migration created in subdirectory Nov 9, 2019
@shroomist
Copy link

seems to work, I'm currently using AnsonT's branch. @wzrdtales could you have a look?

@wzrdtales
Copy link
Member

Thanks for your contribution! 🎉

One thing missing to cover this in the future is a test covering this, could you please add this?

@commitlint-wzrdtales
Copy link

There were the following issues with this Pull Request

  • Commit: 2837aa8
    • ✖ message may not be empty
      , - ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.

🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@AnsonT AnsonT force-pushed the fix-create-migration-scope-path branch from 2837aa8 to ecc41f6 Compare December 3, 2019 17:02
@AnsonT AnsonT force-pushed the fix-create-migration-scope-path branch from ecc41f6 to 2c9e2c4 Compare December 3, 2019 17:03
@AnsonT
Copy link
Contributor Author

AnsonT commented Dec 3, 2019

@wzrdtales

  • Moved createMigrationDir() call to ensure scoped directory is created
  • Added test for creating scoped migration with sql file

@AnsonT AnsonT requested a review from wzrdtales December 13, 2019 03:57
@stale
Copy link

stale bot commented Jan 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 12, 2020
@wzrdtales wzrdtales added the bug label Jan 12, 2020
@stale stale bot removed the stale label Jan 12, 2020
@wzrdtales
Copy link
Member

Looks good, sorry for the delay though :)

@wzrdtales wzrdtales merged commit 57e736c into db-migrate:master Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wzrdtales wzrdtales Awaiting requested review from wzrdtales

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AnsonT @shroomist @wzrdtales