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

Ask for a namespace of when more one path #1414

Merged
merged 1 commit into from
Apr 24, 2024
Merged

Ask for a namespace of when more one path #1414

merged 1 commit into from
Apr 24, 2024

Conversation

Myks92
Copy link
Contributor

@Myks92 Myks92 commented Mar 9, 2024

Q A
Type improvement
BC Break no
Fixed issues no

Summary

When using multiple namespaces with Doctrine Migrations, the first one is selected by default. However, I suggest using an interactive namespace selection mode. This can be very convenient in situations where you have several databases within a modular monolithic application, where the migrations are located in each module.

The improvement affects three a commands:

  • migrations:generate
  • migrations:diff
  • migrations:dump-schema

Example

Global config

# config/packages/doctrine_migrations.yaml

doctrine_migrations:
    migrations_paths:
        'App\Migration': '%kernel.project_dir%/migrations'
    enable_profiler: '%kernel.debug%'
    organize_migrations: BY_YEAR

Modules

// src/Auth/config.php
<?php

declare(strict_types=1);

namespace App\Auth;

return static function (ContainerConfigurator $configurator): void {
    $configurator->extension('doctrine_migrations', [
        'migrations_paths' => [
            'App\\Auth\\Migration' => __DIR__ . '/Migration',
        ],
    ]);
};

Console

Run command without --no-interaction:

php bin/console doctrine:migrations:generate

Снимок экрана 2024-03-09 в 19 20 57

Generated new migration class to path app/src/Auth/Migration with namespace App\Auth\Migration

Run command with --no-interaction:

php bin/console doctrine:migrations:generate --no-interaction

Снимок экрана 2024-03-09 в 19 22 46

Generated new migration class to path app/migrations with namespace App\Migration

@derrabus derrabus changed the base branch from 3.7.x to 3.8.x March 9, 2024 22:11
@derrabus
Copy link
Member

Thank you. Please always write tests that cover your change.

@Myks92
Copy link
Contributor Author

Myks92 commented Mar 12, 2024

Thanks for the comment. I have added tests.

@Myks92
Copy link
Contributor Author

Myks92 commented Mar 25, 2024

@derrabus, what is missing for this PR to be approved?

@derrabus
Copy link
Member

@derrabus, what is missing for this PR to be approved?

A green CI and a code review. I currently don't have time to conduct one, but maybe someone else does.

@Myks92
Copy link
Contributor Author

Myks92 commented Apr 22, 2024

@derrabus, hi! Is there any time for code review? The CI errors are not related to my code. The error is in the file Db Migrator.php which I have not touched. I don't believe it is necessary for me to fix it, but I can if needed.

@greg0ire
Copy link
Member

@Myks92 I addressed that issue for you 🙂

@Myks92
Copy link
Contributor Author

Myks92 commented Apr 22, 2024

@Myks92 I addressed that issue for you 🙂

Thank you for your help! I hope the PR will now be approved.

src/Tools/Console/Command/GenerateCommand.php Outdated Show resolved Hide resolved
tests/Tools/Console/Command/DiffCommandTest.php Outdated Show resolved Hide resolved
tests/Tools/Console/Command/DiffCommandTest.php Outdated Show resolved Hide resolved
tests/Tools/Console/Command/DumpSchemaCommandTest.php Outdated Show resolved Hide resolved
@greg0ire
Copy link
Member

Thank you for your help! I hope the PR will now be approved.

Sorry to disappoint 😬 … but you're on the right track 👍

@Myks92
Copy link
Contributor Author

Myks92 commented Apr 22, 2024

Sorry to disappoint 😬 … but you're on the right track 👍

Thanks for the reviews! I fixed it)

@greg0ire
Copy link
Member

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

  1. git rebase -i origin/3.8.x, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
  2. A window will show up with many lines, replace pick with fixup on every line but the first one
  3. Close your editor, git should do its magic, and you should end up with one commit
  4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

@Myks92
Copy link
Contributor Author

Myks92 commented Apr 23, 2024

@greg0ire It's done!) I know how to do it. Thanks for the detailed instructions!)

@greg0ire
Copy link
Member

greg0ire commented Apr 23, 2024

It's a saved reply, so it's wasn't much effort 😁

@greg0ire greg0ire added this to the 3.8.0 milestone Apr 24, 2024
@greg0ire greg0ire merged commit 7780f8d into doctrine:3.8.x Apr 24, 2024
10 checks passed
@greg0ire
Copy link
Member

Thanks @Myks92 !

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.

4 participants