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

Manual merge of 2.10.x into 2.11.x #4022

Merged
merged 37 commits into from
May 28, 2020
Merged

Manual merge of 2.10.x into 2.11.x #4022

merged 37 commits into from
May 28, 2020

Conversation

greg0ire
Copy link
Member

No description provided.

@greg0ire greg0ire mentioned this pull request May 24, 2020
16 tasks
@greg0ire greg0ire requested a review from morozov May 25, 2020 16:32
@morozov
Copy link
Member

morozov commented May 25, 2020

No, I mean the up-merge and #3977.

@greg0ire
Copy link
Member Author

greg0ire commented May 25, 2020

Oh right, because it's a merge up of #3977 . It's #3977 + changes necessary to make it pass on 2.11.x I won't push it without also pushing #3977 itself though (and the 2 other merges up)

@morozov
Copy link
Member

morozov commented May 25, 2020

Okay. Then should we merge #3977 first? Is it ready for review?

@greg0ire
Copy link
Member Author

@morozov all 4 PRs are ready, don't merge any of them , just approve if they look ok. The plan is described in the TODO list

morozov and others added 23 commits May 26, 2020 22:07
Ignore all violations of the LowercasePHPFunctions sniff in SQLSrvStatement
The latest version is easier to understand for static analyzers.
It is more simple and more accurate
We are testing what happens when providing the wrong type.
The method signature does not allow null.
Not all RDBMS have a concept of database, apparently (see SQLAnywhere).
Those who do not should pass in a string to stay compatible with the
signature.
That alone fixes 6 issues found by Psalm
This is what is expected with FETCH_COLUMN
greg0ire added 14 commits May 27, 2020 22:29
This is precisely what we are testing here.
This hack is temporary and should be removed on 3.0.x
This return type is conditional to $params['wrapperClass']. Luckily,
recent versions of Psalm allow documenting it properly.
See vimeo/psalm#3277

Note that phpstan is not able to understand this yet, but still attempts
to, hence the extra ignore rules.
It confuses both Psalm and PHPStan, and involves complicated mocking.
The php docs are not very clear about that one.
@greg0ire greg0ire changed the title Manual merge of 2.10.x + #3977 into 2.11.x Manual merge of 2.10.x into 2.11.x May 27, 2020
@greg0ire
Copy link
Member Author

@morozov please approve this one (no additional commits here)

@greg0ire greg0ire merged commit ce21834 into doctrine:2.11.x May 28, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants