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 support for PHP 8 #662

Merged
merged 4 commits into from
Feb 1, 2021
Merged

Add support for PHP 8 #662

merged 4 commits into from
Feb 1, 2021

Conversation

lepiaf
Copy link
Contributor

@lepiaf lepiaf commented Dec 8, 2020

No description provided.

@lepiaf
Copy link
Contributor Author

lepiaf commented Dec 8, 2020

waiting for merge of PR doctrine/mongodb-odm#2248

@malarzm
Copy link
Member

malarzm commented Dec 9, 2020

@lepiaf thanks for the initial work! This will need to wait until we release 2.2, but in the meantime maybe you could change ODM to require @alcaeus branch if it's in green-ish state? That could allow you to check if any further work is required

@malarzm malarzm mentioned this pull request Dec 10, 2020
@stale
Copy link

stale bot commented Jan 10, 2021

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Issues that have not seen any activity in 30 days label Jan 10, 2021
@stale stale bot closed this Jan 17, 2021
@alcaeus alcaeus reopened this Jan 18, 2021
@stale stale bot removed the Stale Issues that have not seen any activity in 30 days label Jan 18, 2021
@alexislefebvre alexislefebvre mentioned this pull request Jan 19, 2021
@franmomu
Copy link
Contributor

So there has been a release: https://github.com/doctrine/mongodb-odm/releases/tag/2.2.0 😬

@franmomu
Copy link
Contributor

Just in case, I created yesterday #666.

@lepiaf
Copy link
Contributor Author

lepiaf commented Jan 25, 2021

I have this error in phpunit for PHP 8.

Generating code coverage report in Clover XML format ... failed [00:01.648]
Cannot parse /home/runner/work/DoctrineMongoDBBundle/DoctrineMongoDBBundle/vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/GraphLookup/Match.php: Syntax error, unexpected T_MATCH, expecting T_STRING on line 15

In PHP 8, Match is a reserved keyword. This error comes from doctrine/mongodb-odm library. There is already a deprecation error on it https://github.com/doctrine/mongodb-odm/blob/2.2.0/UPGRADE-2.2.md#aggregation.

This error is raised on code coverage.

@@ -17,5 +17,6 @@ public function testCollector()
$collector = new CommandDataCollector(new CommandLogger());

$collector->collect($request = new Request(['group' => '0']), $response = new Response());
$this->assertSame(0, $collector->getCommandCount());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

test was marked as risky. so I've added at least one assert.

composer.json Outdated Show resolved Hide resolved
@@ -35,18 +35,12 @@ class Guesser
/** @ODM\Field(type="bool") */
public $boolField;

/** @ODM\Field(type="boolean") */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had this error in phpunit

Remaining indirect deprecation notices (2)

  1x: Since doctrine/mongodb-odm 2.1: The "boolean" mapping type is deprecated. Use "bool" instead.
    1x in TypeGuesserTest::testTypesShouldBeGuessedCorrectly from Doctrine\Bundle\MongoDBBundle\Tests\Form\Type

  1x: Since doctrine/mongodb-odm 2.1: The "integer" mapping type is deprecated. Use "int" instead.
    1x in TypeGuesserTest::testTypesShouldBeGuessedCorrectly from Doctrine\Bundle\MongoDBBundle\Tests\Form\Type

Copy link
Member

Choose a reason for hiding this comment

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

It'd be the best to extract these two asserts into a separate test that would allow deprecation notices but considering simplicity of the guesser I think we'll be just fine

@lepiaf lepiaf marked this pull request as ready for review January 25, 2021 22:09
@malarzm malarzm merged commit 3cc1392 into doctrine:4.3.x Feb 1, 2021
@malarzm
Copy link
Member

malarzm commented Feb 1, 2021

Thanks a lot @lepiaf!

@malarzm malarzm added this to the 4.3.0 milestone Feb 1, 2021
@lepiaf lepiaf deleted the support-php8 branch February 2, 2021 07:20
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