Manually merge 2.11.x into 3.0.x #4275

greg0ire · 2020-09-19T10:10:02Z

Not gonna lie, this was super hard because of:

the exception rename
the sqlite thingy revert
things related to exceptions that happened in the meantime on 3.0.x

Please review super carefully. If it turns out not to be correct and too hard to fix, let's try doing the items above one by one. I used imerge for the merge, which helped a bit I think, but still…

Update PHPStan to 0.12.40

Rename - sp_RENAME to sp_rename; - SysObjects to sysobjects; - SysColumns to syscolumns. Fixes issues with case sensitive collation on mssql database.

Fix errors with case sensitive collation on mssql: not existing procedures and tables (sp_RENAME, SysObjects, SysColumns)

…LATION

…on testing

Cleanup Travis scripts and configuration

If there is no constraint name defined the listTableForeignKeys should not invent a new one as a numerical id has no deeper meaning and makes schema changes adding information that was not there before. Using `null` is an apropriate value as its already in use when handling not existing fk names.

SQLite: Fix wrongly detected reference constraint name on schema change

This is part of an effort to standardize workflows accross repositories. We do not have a way to do it automatically yet, but it is still nice to have one reference repository.

Reuse global coding standard workflow

Deprecate DBAL\DBALException in favor of DBAL\Exception

Handle both ways of PDO reporting failed connection

This reverts commit f0c1af4.

This reverts commit 85a983c.

…ema change" This reverts commit 7570aeb.

Revert full support for foreign key constraints for SQLite

Setup automatic release workflow

Closes doctrine#4263

greg0ire · 2020-09-19T16:33:56Z

@morozov as outlined in #4274 (comment), I'm not merging 2.11.x here, but a prior state of it, so I believe the correct diff to compare against would be ce85bd8...cce615a

I could have done all steps in one go, but since step 1 is super complicated already, I figured it would be easier to review like this.

greg0ire · 2020-09-19T17:04:05Z

Here are the 4 classes that do not appear in the diff of this PR, but do appear in the diff I linked:

AbstractSQLAnywhereDriver: does not exist on 3.0.x
AbstractSQLServerDriver: does not throw on 3.0.x
AbstractSQLiteDriver: already does not have the code that gets removed
Mysqli/Driver: no longer throws a DBALException

morozov · 2020-09-19T17:35:44Z

OK, so the PR is only expected to not include the changes from #4274 but the second chunk mentioned in the review comment doesn't belong there. Honestly, I don't see much benefit in taking the merge work apart. It adds confusion and complexity and doesn't reduce the amount of work to be done.

Why don't we merge everything at once as we've been doing so far?

greg0ire · 2020-09-19T17:47:02Z

Why don't we merge everything at once as we've been doing so far?

I thought it would be best to do that because steps 2 and 3 of the plan I described make this non-trivial.

the second chunk mentioned in the review comment doesn't belong there

not sure what you are talking about:

Manually merge 2.11.x into 3.0.x #4275 (comment) ? in this case it's the chunk mentioned in the second review comment, and I removed it.
ce85bd8...2.11.x#diff-51b2b592af5d0b614210f1e1f9d5eeb0 ? You said that one was missing from the diff, so how can it "not belong here"?

I will try the next steps, hopefully it will make things easier.

morozov · 2020-09-19T17:53:04Z

The chunk missing from the current PR (the changes in AbstractPostgreSQLDriver from #4268) does not belong to #4274, so it should be here but it doesn't seem to.

greg0ire · 2020-09-19T18:05:42Z

Please compare the diff above and the PR and see if something else is missing.

Ok I think I understood where I messed up. Will tell you when I am done with that.

greg0ire · 2020-09-19T18:20:35Z

Done, please resume your review.

morozov · 2020-09-20T01:45:45Z

Now this PR introduces the getAsciiStringTypeDeclarationSQL() method from #4274 but via the merge commit instead of having 7b58fd2 in the ancestry tree.

greg0ire · 2020-09-20T07:46:14Z

@morozov fixed, sorry

morozov · 2020-09-20T14:30:01Z

This is still not “Merge 2.11.x into 3.0.x”. The 0d9f0ad commit doesn't belong to 2.11.x, and there's 3 merge commits at the end.

Please redo it the normal way. Instead of resolving the conflicts again, you can use the tree in its current state.

greg0ire · 2020-09-20T15:17:17Z

The 0d9f0ad commit doesn't belong to 2.11.x

This was on purpose. Since gjdanis had already done the work for 3.0.x , I thought it would be simpler to merge this work, which 0d9f0ad represents, and then use the ours strategy to let git know that this is equivalent to merging what gjdanis did on 2.11.x . The 3 merge commits are:

merge 2.11.x as it was before gjdanis' contribution
merge gjdanis' contribution as it was before you rebased it on 2.11.x
merge -s ours 2.11.x current state.

Instead of resolving the conflicts again, you can use the tree in its current state.

Sounds easy enough. Done.

morozov · 2020-09-20T15:24:46Z

Since gjdanis had already done the work for 3.0.x , I thought it would be simpler to merge this work, which 0d9f0ad represents, and then use the ours strategy to let git know that this is equivalent to merging what gjdanis did on 2.11.x .

I get the idea but it's not “Merge … into …”, it's something else which we don't normally do. It could be done behind the curtains and documented since it's an implementation detail of the PR, not the intent.

Apart from that, I made a couple of changes during the backport, so if just the original commit was used for merging up, there should have been another one on top (which you had I believe). This whole dance complicates the process hugely and unnecessarily IMO.

src/Driver/PDO/SQLSrv/Statement.php

tests/Schema/SchemaDiffTest.php

morozov · 2020-09-20T18:27:52Z

The issue in the SchemaDiffTest must be the last one. The rest looks good.

morozov · 2020-09-20T19:46:28Z

Not gonna lie, this was super hard

Thanks, @greg0ire!

morozov and others added 30 commits August 26, 2020 21:32

Update PHPStan to 0.12.40

4bc4569

Merge pull request doctrine#4235 from morozov/phpstan-0.12.40

1efdc63

Update PHPStan to 0.12.40

use proper case for mssql names

78547c7

Rename - sp_RENAME to sp_rename; - SysObjects to sysobjects; - SysColumns to syscolumns. Fixes issues with case sensitive collation on mssql database.

Add Travis job with case sensitive mssql database

70e1f95

Merge pull request doctrine#4238 from redgnar/2.10.x

dd6c756

Fix errors with case sensitive collation on mssql: not existing procedures and tables (sp_RENAME, SysObjects, SysColumns)

Parametrize SQL Server installation script, add handling of MSSQL_COL…

05d55f6

…LATION

Reuse PHP 7.4 sqlsrv and pdo_sqlsrv builds for case-sensitive collati…

354322e

…on testing

Reuse MySQL 8 build for TLS testing

f27faa3

Merge pull request doctrine#4239 from morozov/travis-cleanup

d0bde40

Cleanup Travis scripts and configuration

Release 2.10.3

03ca23a

Merge branch '2.10.x' into 2.11.x

f69c990

Merge pull request doctrine#4246 from MyIgel/patch-1

cd2ddda

SQLite: Fix wrongly detected reference constraint name on schema change

Remove redundant condition

6c2d629

Clean up exception handling in Connection

a2ec3ab

Reuse global coding standard workflow

16d5861

This is part of an effort to standardize workflows accross repositories. We do not have a way to do it automatically yet, but it is still nice to have one reference repository.

Remove redundant @throws annotations from test methods

c115e9f

Deprecate DBAL\DBALException in favor of DBAL\Exception

ad1e482

Merge pull request doctrine#4252 from greg0ire/sync-phpcs-workflow

f392ffc

Reuse global coding standard workflow

Merge pull request doctrine#4253 from morozov/deprecate-dbal-exception

f97ee94

Deprecate DBAL\DBALException in favor of DBAL\Exception

Handle both ways of PDO reporting failed connection

34f3ced

Merge pull request doctrine#4268 from morozov/issues/4267

fdbd770

Handle both ways of PDO reporting failed connection

Revert "Use proper check in acceptForeignKey()"

3caa55c

This reverts commit f0c1af4.

Revert "Add full support for foreign key constraints for SQLite"

baec4b8

This reverts commit 85a983c.

Revert "SQLite: Fix wrongly detected reference constraint name on sch…

912db7a

…ema change" This reverts commit 7570aeb.

Merge pull request doctrine#4255 from morozov/issues/4243

03bc93b

Revert full support for foreign key constraints for SQLite

Merge branch '2.10.x' into 2.11.x

76c4c33

Setup automatic release workflow

d333f7c

Merge pull request doctrine#4251 from doctrine/greg0ire-patch-1

cce615a

Setup automatic release workflow

Support ASCII parameter binding

7b58fd2

Closes doctrine#4263

greg0ire force-pushed the 3.0.x branch from 62146d0 to 2d69aef Compare September 19, 2020 16:35

greg0ire changed the title ~~Manually merge cce615a9e into 3.0.x~~ Manually merge 2.11.x into 3.0.x Sep 19, 2020

greg0ire force-pushed the 3.0.x branch from 3271507 to b6a3597 Compare September 19, 2020 18:04

greg0ire requested a review from morozov September 19, 2020 18:20

greg0ire force-pushed the 3.0.x branch from b6a3597 to 5d23cd1 Compare September 20, 2020 07:46

greg0ire force-pushed the 3.0.x branch from 5d23cd1 to c342515 Compare September 20, 2020 15:17

morozov reviewed Sep 20, 2020

View reviewed changes

src/Driver/PDO/SQLSrv/Statement.php Outdated Show resolved Hide resolved

greg0ire force-pushed the 3.0.x branch from c342515 to aad7e30 Compare September 20, 2020 16:19

morozov reviewed Sep 20, 2020

View reviewed changes

tests/Schema/SchemaDiffTest.php Outdated Show resolved Hide resolved

morozov added the Merge Up label Sep 20, 2020

Merge remote-tracking branch 'origin/2.11.x' into 3.0.x

789d900

morozov force-pushed the 3.0.x branch from aad7e30 to 789d900 Compare September 20, 2020 19:33

morozov approved these changes Sep 20, 2020

View reviewed changes

morozov merged commit 789d900 into doctrine:3.0.x Sep 20, 2020

morozov mentioned this pull request Sep 21, 2020

Remove deprecated APIs #4278

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manually merge 2.11.x into 3.0.x #4275

Manually merge 2.11.x into 3.0.x #4275

greg0ire commented Sep 19, 2020

greg0ire commented Sep 19, 2020 •

edited

Loading

greg0ire commented Sep 19, 2020

morozov commented Sep 19, 2020

greg0ire commented Sep 19, 2020

morozov commented Sep 19, 2020

greg0ire commented Sep 19, 2020

greg0ire commented Sep 19, 2020

morozov commented Sep 20, 2020

greg0ire commented Sep 20, 2020

morozov commented Sep 20, 2020 •

edited

Loading

greg0ire commented Sep 20, 2020

morozov commented Sep 20, 2020 •

edited

Loading

morozov commented Sep 20, 2020

morozov commented Sep 20, 2020

Manually merge 2.11.x into 3.0.x #4275

Manually merge 2.11.x into 3.0.x #4275

Conversation

greg0ire commented Sep 19, 2020

greg0ire commented Sep 19, 2020 • edited Loading

greg0ire commented Sep 19, 2020

morozov commented Sep 19, 2020

greg0ire commented Sep 19, 2020

morozov commented Sep 19, 2020

greg0ire commented Sep 19, 2020

greg0ire commented Sep 19, 2020

morozov commented Sep 20, 2020

greg0ire commented Sep 20, 2020

morozov commented Sep 20, 2020 • edited Loading

greg0ire commented Sep 20, 2020

morozov commented Sep 20, 2020 • edited Loading

morozov commented Sep 20, 2020

morozov commented Sep 20, 2020

greg0ire commented Sep 19, 2020 •

edited

Loading

morozov commented Sep 20, 2020 •

edited

Loading

morozov commented Sep 20, 2020 •

edited

Loading