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

Add a migrations command #709

Merged
merged 36 commits into from
Jan 6, 2020
Merged

Add a migrations command #709

merged 36 commits into from
Jan 6, 2020

Conversation

ausi
Copy link
Member

@ausi ausi commented Sep 2, 2019

See contao/core-bundle#1223 (comment)

Todo:

  • Tests
  • Use migrations service in the install tool

@ausi ausi added the feature label Sep 2, 2019
@ausi ausi added this to the 4.9 milestone Sep 2, 2019
@ausi ausi self-assigned this Sep 2, 2019
ausi and others added 13 commits December 8, 2019 11:48
Co-Authored-By: Hannes <xchs@gmx-topmail.de>
Co-Authored-By: Hannes <xchs@gmx-topmail.de>
Conflicts:
	core-bundle/src/ContaoCoreBundle.php
	core-bundle/src/DependencyInjection/ContaoCoreExtension.php
	core-bundle/src/Resources/config/services.yml
	installation-bundle/src/Database/Version330Update.php
	installation-bundle/src/Database/Version350Update.php
	installation-bundle/src/Database/Version400Update.php
	installation-bundle/src/Database/Version410Update.php
	installation-bundle/src/Database/Version430Update.php
	installation-bundle/src/Database/Version440Update.php
	installation-bundle/src/Database/Version447Update.php
	installation-bundle/src/Database/Version450Update.php
	installation-bundle/src/Database/Version460Update.php
	installation-bundle/src/Database/Version470Update.php
	installation-bundle/src/Database/Version480Update.php
	installation-bundle/src/Resources/config/services.yml
@ausi ausi marked this pull request as ready for review December 14, 2019 20:49
@ausi
Copy link
Member Author

ausi commented Dec 22, 2019

I don't like Migrations either. Imho this is a MigrationCollection and the method should be named runAll().

Renamed in fc52190. runAll() would be misleading IMO because only pending migrations get exectued, not all. I renamed to run() now, runPending() would be OK too I think.

Should we add a deprecation notice to the AbstractVersionUpdate class?

Done in 23ec58b.

I think all comments should be addressed now. @aschempp please take a look at your review comments and mark them as resolved if you agree with my changes.

@ausi ausi mentioned this pull request Dec 22, 2019
2 tasks
@ausi
Copy link
Member Author

ausi commented Dec 22, 2019

please also provide a PR for the documentation :)

Docs pull request: contao/docs#205

@leofeyer leofeyer changed the title Add migrations command Add a migrations command Dec 23, 2019
Toflar
Toflar previously approved these changes Jan 3, 2020
aschempp
aschempp previously approved these changes Jan 3, 2020
Copy link
Member

@aschempp aschempp left a comment

Choose a reason for hiding this comment

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

I would suggest to use the FQCN instead of the generated name, but I‘m also fine with merging as is 👍🏻

@ausi ausi dismissed stale reviews from aschempp and Toflar via a6d2b26 January 3, 2020 17:10
@ausi
Copy link
Member Author

ausi commented Jan 4, 2020

I would suggest to use the FQCN instead of the generated name

Done in a6d2b26

Toflar
Toflar previously approved these changes Jan 4, 2020
@leofeyer leofeyer merged commit a2d0e52 into master Jan 6, 2020
@leofeyer leofeyer deleted the feature/migrate-command branch January 6, 2020 15:23
@leofeyer
Copy link
Member

leofeyer commented Jan 6, 2020

Thank you @ausi.

@Toflar
Copy link
Member

Toflar commented Jan 6, 2020

This is biiiiig!! ❤️ 🎉

@bytehead
Copy link
Member

bytehead commented Jan 6, 2020

Thank you big time @ausi! 😍

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.

None yet

7 participants