platformsh / symfonyflex-bridge

Bridge library for running Symfony Flex on Platform.sh
MIT License
30 stars 15 forks source link

Use database type env exposure to dynamically set the DB type for Doctrine. #12

Closed tristanbes closed 5 years ago

tristanbes commented 5 years ago

PostgreSQL latest version on platform.sh is version 11. Source.

It seems a little bit weird to hardcode this value when we can access it through PLATFORM_RELATIONSHIPS

Edit: example of the algorithm: https://3v4l.org/q70OT

Crell commented 5 years ago

Thanks for the PR. The hard-coding is because this code predates when we were reliably exposing the version numbers in the first place, so it's good to get that cleaned up.

Of note: Compliance with PHP < 7 is completely irrelevant, as Symfony 4 requires PHP 7.1 anyway, as does this library due to the return types. (For completeness that should probably get added to the composer.json file too, but it's never going to be an issue due to Symfony 4's requirements.) So go all out with PHP 7 goodness.

tristanbes commented 5 years ago

ping @Crell

Crell commented 5 years ago

The test suite needs to get updated, too. I get 1 error and 2 failures from it.

1) Platformsh\FlexBridge\Tests\FlexBridgeDatabaseTest::testDatabaseRelationshipSet
Undefined index: type

/home/crell/Platformsh/symfonyflex-bridge/platformsh-flex-env.php:100
/home/crell/Platformsh/symfonyflex-bridge/platformsh-flex-env.php:32
/home/crell/Platformsh/symfonyflex-bridge/tests/FlexBridgeDatabaseTest.php:84

--

There were 2 failures:

1) Platformsh\FlexBridge\Tests\FlexBridgeDatabaseTest::testNoRelationships
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'mysql://:@localhost:3306/?charset=utf8mb4&serverVersion=10.2'
+'mysql://:@localhost:3306/?charset=utf8mb4&serverVersion=mariadb-10.2.12'

/home/crell/Platformsh/symfonyflex-bridge/tests/FlexBridgeDatabaseTest.php:62

2) Platformsh\FlexBridge\Tests\FlexBridgeDatabaseTest::testNoDatabaseRelationship
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'mysql://:@localhost:3306/?charset=utf8mb4&serverVersion=10.2'
+'mysql://:@localhost:3306/?charset=utf8mb4&serverVersion=mariadb-10.2.12'

/home/crell/Platformsh/symfonyflex-bridge/tests/FlexBridgeDatabaseTest.php:76

(I should probably add Travis integration to this project at some point, too.)

Crell commented 5 years ago

I just added a Travis CS file for this project in the master branch. Please merge that to your branch so this PR gets automatically tested. Thanks.

tristanbes commented 5 years ago

@Crell, Tests are green, thanks for providing them. Also, I've cleaned the commit history.

(Coding from straight from Github is really the worst idea ever. 😱)

Crell commented 5 years ago

Coding from straight from Github is really the worst idea ever.

It really is. :smile: (For anything but the most trivial typos; and sometimes even then.)

Thanks for carrying this home! Merging.