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

feat: Allow installing and testing with all database types, including MariaDB #34

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

rfay
Copy link
Contributor

@rfay rfay commented May 2, 2024

(Draft: This has to be rebased after

This PR allows using any database type supported by DDEV, including MariaDB and MySQL. And of course it can still use sqlite.

It currently includes unmerged PRs because drush is required for it.

I guess it still needs tests, will wait until after rebase.

@joachim-n
Copy link

@rfay Could you rebase this against main please? There are a lot of conflicts and I don't know what I'm doing with the code involved.

@rfay
Copy link
Contributor Author

rfay commented Oct 2, 2024

I can rebase this and #33, but it will still be dependent on #33

@joachim-n
Copy link

Ah drat, I hadn't seen this depended on #33. Too many balls in the air, sorry!

@joachim-n
Copy link

I don't know if in my fork I want to support both styles of installation, or just using my Composer project template. I was initially thinking I'd support both, if it's not too much faff.

@rfay rfay force-pushed the 20240502_mariadb_support branch from 0d8d9b1 to 90eeb91 Compare October 2, 2024 18:25
@rfay
Copy link
Contributor Author

rfay commented Oct 2, 2024

This is rebased against

But of course it still depends on #33.

@rfay
Copy link
Contributor Author

rfay commented Oct 2, 2024

I don't know if in my fork I want to support both styles of installation, or just using my Composer project template. I was initially thinking I'd support both, if it's not too much faff.

All I care about here is using a "normal" database type, and having drush available. I'm pretty sure your template is a fine way to do that, true?

@joachim-n
Copy link

Yup, but what I mean is if I drop support for running this directly in a Drupal core clone, then none of #33 is needed.

@rfay
Copy link
Contributor Author

rfay commented Oct 2, 2024

Again, any way you get drush works for me :)

;;
esac
fi
perl -pi -e "s|SIMPLETEST_DB_VALUE|${dburl}|g" core/phpunit.xml

Choose a reason for hiding this comment

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

@rfay is it intentional that this is still using the sqlite DB for running PHPUnit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this was using mariaDB by accident until a recent PR. But IMO it should use mariaDB by default and allow using any supported database. That's actually why I started working on this PR a bit, because I wanted to see "real" performance comparisons on Drupal, comparing DB types. I think it's possible that the use of sqlite3 "for performance reasons" may be obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants