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

Allow Symfony 6 #9

Merged
merged 4 commits into from
Jan 24, 2022
Merged

Allow Symfony 6 #9

merged 4 commits into from
Jan 24, 2022

Conversation

ddeboer
Copy link
Contributor

@ddeboer ddeboer commented Jan 7, 2022

* Require doctrine/migrations 3.1.2 for PSR-12 formatting.
@ddeboer
Copy link
Contributor Author

ddeboer commented Jan 10, 2022

@mnapoli This should be green now.

@mnapoli
Copy link
Owner

mnapoli commented Jan 10, 2022

I don't understand why that error is popping up 🤔

If that makes things easier we can also drop PHP 7.4? Not sure if that would help here.

@ddeboer
Copy link
Contributor Author

ddeboer commented Jan 10, 2022

@mnapoli Didn’t make sense to me either, so dropping PHP 7.4 support.

@ddeboer
Copy link
Contributor Author

ddeboer commented Jan 17, 2022

Hey @mnapoli, do you agree with this change? If so, would be cool to have this merged & tagged.

@@ -29,6 +29,6 @@ jobs:
tools: composer:v2
coverage: none
- name: Install PHP dependencies
run: composer update ${{ matrix.dependency-version }} --prefer-dist --no-interaction --no-progress --no-suggest --ignore-platform-reqs
run: composer update ${{ matrix.dependency-version }} --prefer-dist --no-interaction --no-progress --ignore-platform-req=ext-pdo
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don’t ignore the PHP version when installing dependencies to prevent doctrine/dbal version being installed that is incompatible with PHP 8.0. Only ignore the PDO requirement.

Also, --no-suggest is deprecated, so remove it.

@mnapoli
Copy link
Owner

mnapoli commented Jan 22, 2022

Yes I'm 👍 to merge this, tests are still causing issue though

@ddeboer
Copy link
Contributor Author

ddeboer commented Jan 22, 2022

@mnapoli Hmm, that seems to be a result of the release of doctrine/migrations 3.4, which changed the formatting of the migration file (again).

How important is it for this library to test $migration->getDiffGenerator()->generate(); would $migration->getMigrationSqlGenerator()->generate() also suffice?

@mnapoli
Copy link
Owner

mnapoli commented Jan 22, 2022

I guess it would work as long as it tests the feature 😄

@ddeboer
Copy link
Contributor Author

ddeboer commented Jan 24, 2022

Changed the test to only compare the contents of the SQL statements, ignoring any changes in the migration’s PHP code.

@mnapoli
Copy link
Owner

mnapoli commented Jan 24, 2022

Thank you for your patience :)

@mnapoli mnapoli merged commit 422ed1b into mnapoli:master Jan 24, 2022
@ddeboer ddeboer deleted the symfony-6 branch January 24, 2022 12:21
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

2 participants