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

bake migration_diff now supports --source for multiple database configs #618

Merged
merged 9 commits into from
Sep 22, 2023

Conversation

telepathPiddlingAccent
Copy link
Contributor

Implemented a souce option for bake migration_diff. migration_diff now supports multiple target folders for the generated migration.

This is especially what was requested in issue #608 and it also bothered our company.

This has now been built on top of the 3.x branch (because we need it theres) but should probably be merged to 4.x as well.

We added tests for the option and also executed CS before opening the PR, both are running without errors.

We appreciate your feedback if there is more to change to be compliant with the contribution guidelines.

src/Command/BakeSimpleMigrationCommand.php Outdated Show resolved Hide resolved
src/Command/BakeSimpleMigrationCommand.php Show resolved Hide resolved
src/Command/SnapshotTrait.php Show resolved Hide resolved
tests/TestCase/Command/BakeMigrationDiffCommandTest.php Outdated Show resolved Hide resolved
Comment on lines 68 to 72
$path = ROOT . DS . $this->pathFragment;
$path = ROOT . DS . $this->pathFragment . DS . $args->getOption('source') . DS;
if ($this->plugin) {
$path = $this->_pluginPath($this->plugin) . $this->pathFragment;
$path = $this->_pluginPath($this->plugin) . $this->pathFragment . DS . $args->getOption('source') . DS;
Copy link
Member

Choose a reason for hiding this comment

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

$args->getOption('source') could be extracted into an intermediate variable to improve readability and reduce redunant calls.

…tion('source') . DS' into an intermediate variable
@othercorey
Copy link
Member

Is there anything blocking this?

@othercorey othercorey added this to the 3.x (CakePHP 4) milestone Sep 22, 2023
@othercorey othercorey merged commit 58446fd into cakephp:3.x Sep 22, 2023
10 checks passed
@othercorey
Copy link
Member

Released in Migrations 3.9, thanks!

ndm2 added a commit that referenced this pull request Sep 22, 2023
#618 changed how the path for the baked files is being built.
@Rakasch
Copy link

Rakasch commented Oct 10, 2023

Does not seem to work for me.
Just updated the packages to get migrations v3.9.
Does it not work for Cake 4.4.x ? Or what am I missing?

cakephp/cakephp: 4.4.18,
cakephp/migrations: 3.9.0,

image

@ndm2
Copy link
Contributor

ndm2 commented Oct 10, 2023

@Rakasch You are using bake model there, not bake migration_diff.

@Rakasch
Copy link

Rakasch commented Oct 11, 2023

@ndm2
oh yes thanks. Got some things mixed up. Evil vacation made my brain confused 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants