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

Remove migrations command #1473

Merged
merged 2 commits into from
Jul 20, 2022

Conversation

eerison
Copy link
Contributor

@eerison eerison commented Jul 19, 2022

Remove migrations command

I am targeting this branch, because those commands are final and not used anymore, check here #1465 (comment) for more details

Closes #1465.

Changelog

### Removed
- Removed the command `MigrateBlockNameSettingCommand::class`
- Removed the command `MigrateToJsonTypeCommand::class`

@eerison eerison mentioned this pull request Jul 19, 2022
1 task
jordisala1991
jordisala1991 previously approved these changes Jul 19, 2022
@jordisala1991
Copy link
Member

Do we need an upgrade note here @VincentLanglet ?

@VincentLanglet
Copy link
Member

Do we need an upgrade note here @VincentLanglet ?

I would say no but we need to deprecate this command on 3.x first ; or am I missing something ?

@eerison
Copy link
Contributor Author

eerison commented Jul 19, 2022

Do we need an upgrade note here @VincentLanglet ?

I would say no but we need to deprecate this command on 3.x first ; or am I missing something ?

Even the class as final do we need to deprecate before to remove?

@VincentLanglet
Copy link
Member

Do we need an upgrade note here @VincentLanglet ?

I would say no but we need to deprecate this command on 3.x first ; or am I missing something ?

Even the class as final do we need to deprecate before to remove?

People can still inject this class somewhere else.
And since this is Commands, people can execute them. So we should trigger a deprecation when someone run them.

In a general way, every removal need to be deprecated before ^^

@eerison
Copy link
Contributor Author

eerison commented Jul 20, 2022

Do we need an upgrade note here @VincentLanglet ?

I would say no but we need to deprecate this command on 3.x first ; or am I missing something ?

Even the class as final do we need to deprecate before to remove?

People can still inject this class somewhere else. And since this is Commands, people can execute them. So we should trigger a deprecation when someone run them.

In a general way, every removal need to be deprecated before ^^

True! I didn't thought that, I'll deprecate first ;)

@eerison
Copy link
Contributor Author

eerison commented Jul 20, 2022

I opened the PR :)

#1476

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@VincentLanglet
Copy link
Member

Let's wait for the merge of #1476 and 3.x into 4.x then

@VincentLanglet
Copy link
Member

Done, you can update @eerison

 into remove_migration_commands

� Conflicts:
�	src/Command/MigrateBlockNameSettingCommand.php
�	src/Command/MigrateToJsonTypeCommand.php
@eerison
Copy link
Contributor Author

eerison commented Jul 20, 2022

Done, you can update @eerison

Done :)

@VincentLanglet VincentLanglet merged commit 99c9493 into sonata-project:4.x Jul 20, 2022
@eerison eerison deleted the remove_migration_commands branch July 20, 2022 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants