-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Allow other schema than "public" on PostgreSQL databases #217
Allow other schema than "public" on PostgreSQL databases #217
Conversation
@wilsonge Does |
Yes :) |
@wilsonge Do you know who currently maintains this repo? And does it need an extra PR for 2.0-dev, or will this one be merged up after it has been merged into master? |
@richard67 Llewellyn and I have taken over the responsibility for the framework. There's still a lot we have figure out, so please bear with us ;) If the merge wents through, we'll merge it; otherwise, we'll get back to you. Nevertheless, there are failing tests, which need to be fixed, before this PR can be merged:: There was 1 error:
1) Joomla\Database\Tests\DriverPostgresqlTest::testInsertObject
pg_prepare(): Query failed: ERROR: syntax error at or near ")"
LINE 2: () VALUES
^
C:\projects\database\src\Postgresql\PostgresqlDriver.php:961
C:\projects\database\src\Postgresql\PostgresqlDriver.php:1276
C:\projects\database\Tests\DriverPostgresqlTest.php:595
--
There was 1 failure:
1) Joomla\Database\Tests\DriverPostgresqlTest::testGetTableColumns
315
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
- 'id' => 'integer'
- 'title' => 'character varying'
- 'start_date' => 'timestamp without time zone'
- 'description' => 'text'
- 'data' => 'bytea'
)
C:\projects\database\Tests\DriverPostgresqlTest.php:315 |
@nibra If you check some other PR for the master branch you will see the same test failing. So it's not related to this PR. I've tried to find the reason, but I wasn't successful. |
@wilsonge Can you confirm? |
I'd wait for Harald to build joomla/joomla-cms#30570 before merging. But this is fine as a backport of that work otherwise. @richard67 it will be merged up to 2.0-dev when this is merged |
@richard67 looks like #218 pr to master is passing - so think the tests failing are probably coming from these changes :/ |
@wilsonge Others have travis-ci and not appveyour. Why that? |
@wilsonge Found my mistake, had forgotten a part. |
@nibra Found my mistake, should be fixed now. |
@richard67 Looks much better! Let's wait for Harald to merge joomla/joomla-cms#30570, as @wilsonge suggested, and then merge this. |
Have changed that in the open CMS PR. |
Now as joomla/joomla-cms#30570 has been merged in the CMS repo, this one here can be merged too, and then merged up from master into 2.-dev. |
Now it needs to be merged up into 2.0-dev here, so someone (I?) can make a PR for the 4.0-dev branch of the CMS to update the database package. |
Pull Request for Issue #157 (part 2)
Summary of Changes
Port back joomla/joomla-cms#24999 and joomla/joomla-cms#30570 to the framework.
Testing Instructions
See testing instructions of joomla/joomla-cms#24999 and joomla/joomla-cms#30570 .
Documentation Changes Required
No idea.