Skip to content

Conversation

WyriHaximus
Copy link
Member

@WyriHaximus WyriHaximus commented Mar 21, 2024

This change set introduces a rdbms matrix with the purpose of increasing insurance this package works with up to the latest MySQL/MariaDB versions.

Note that due to MySQL's versioning there are no v6 and v7.

Refs: #194

@WyriHaximus WyriHaximus added this to the v0.7.0 milestone Mar 21, 2024
@WyriHaximus WyriHaximus force-pushed the 0.7.x-add-rdbms-matrix-for-wider-compatibility-testing branch 5 times, most recently from 3d2d16b to b9f6703 Compare March 21, 2024 21:01
@WyriHaximus WyriHaximus requested review from SimonFrings and clue March 21, 2024 21:03
Copy link
Contributor

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

Interesting approach, but I'm not sure if we should add this to the same MySQL job here. You suggested to test this with mariadb:10 across all supported PHP versions, but I don't think this is necessary here. Looking further, if we want to support additional databases in different versions, this would result in a huge (and completely confusing) test matrix in GitHub actions.

I think we should test mysql across all supported PHP versions, but for mariadb we could also test against the latest PHP version(s) in a different job to avoid overloading this.

What do you think?

@WyriHaximus
Copy link
Member Author

Interesting approach, but I'm not sure if we should add this to the same MySQL job here. You suggested to test this with mariadb:10 across all supported PHP versions, but I don't think this is necessary here. Looking further, if we want to support additional databases in different versions, this would result in a huge (and completely confusing) test matrix in GitHub actions.

We could introduce MariaDB in a follow up PR.

I think we should test mysql across all supported PHP versions, but for mariadb we could also test against the latest PHP version(s) in a different job to avoid overloading this.

Givin that MariaDB is a fork by the creator on MySQL, we could flip a could and prefer one of them?

What do you think?

To be brutally honest: Test all permutations; but that is a personal preference that I enforce in my own packages and projects, so I'll make the adjustment for this PR.

@WyriHaximus WyriHaximus force-pushed the 0.7.x-add-rdbms-matrix-for-wider-compatibility-testing branch from b9f6703 to 249c5fe Compare April 23, 2024 13:09
@WyriHaximus WyriHaximus requested a review from SimonFrings April 23, 2024 13:10
@WyriHaximus
Copy link
Member Author

@SimonFrings Pushed the changes

This change set introduces a `rdbms` matrix with the purpose of increasing insurance this package works with up to the latest MySQL/MariaDB versions.

Note that due to MySQL's versioning there are no `v6` and `v7`.

Refs: friends-of-reactphp#194
@WyriHaximus WyriHaximus force-pushed the 0.7.x-add-rdbms-matrix-for-wider-compatibility-testing branch from 23da9a7 to 00f1a0c Compare May 21, 2025 16:27
Copy link
Contributor

@clue clue left a comment

Choose a reason for hiding this comment

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

@WyriHaximus Thanks for looking into this, changes LGTM! :shipit:

Especially useful once #207 is merged for MySQL 8 and MySQL 9 support! 🎉

@clue clue merged commit 84f6cbb into friends-of-reactphp:0.7.x May 21, 2025
15 checks passed
@WyriHaximus WyriHaximus deleted the 0.7.x-add-rdbms-matrix-for-wider-compatibility-testing branch May 21, 2025 16:32
@WyriHaximus
Copy link
Member Author

@clue Cheers, having a look at 206 and 207 in the next hour 👍

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.

3 participants