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

Fix installer when database not exists #6739

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nicosomb
Copy link
Member

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Documentation no
Translation no
CHANGELOG.md no
License MIT

I'd prefer have a regex to check the whole exception message. If someone wants to have a look on it.

My PR is a pointer to the bug, we can improve it.

I want to kill an other bug quickly #6659

@nicosomb nicosomb requested review from j0k3r and Kdecherf July 19, 2023 13:37
Copy link
Member

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

Maybe you can add an extra build without creating the MySQL or PG database first to see if it fix the problem

Copy link
Member

Choose a reason for hiding this comment

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

What is that file?

@nicosomb
Copy link
Member Author

All is green but the CI is broken 😉

@@ -151,3 +151,65 @@ jobs:

- name: "Run PHPUnit"
run: "php bin/simple-phpunit -v"

phpunit_no_database:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of running these new tests on all PR as it unnecessarily increases load on the CI. @nicosomb could we disable them by default (or drop the commit before merging)?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can just keep one? For MySQL or PG

@nicosomb
Copy link
Member Author

@j0k3r can you help me about the new build to create please?

@j0k3r
Copy link
Member

j0k3r commented Aug 24, 2023

@nicosomb the build seems ok now, but tests are failing

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

3 participants