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.

Sign up for GitHub

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

Jump to bottom

Merge 2.12.x up into 3.0.x #4475

Merged
merged 20 commits into from
Jan 8, 2021
Merged

Merge 2.12.x up into 3.0.x #4475

merged 20 commits into from
Jan 8, 2021

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Jan 7, 2021

No description provided.

kishore-hariram and others added 15 commits November 28, 2020 10:47
@kishore-hariram @morozov
No need of comma
c098382
@morozov
Merge pull request doctrine#3682 from kikilass/master
ecadac4 
No need of comma
@morozov
Remove unmaintained branches from the project metadata
a7b5f09
@morozov
Merge pull request doctrine#4453 from morozov/remove-unmaintained-bra…
98233ab 
…nches

Remove unmaintained branches from the project metadata
@trusek @morozov
MSSQL adds a redundant ORDER BY clause when with subquery with ORDER BY
7aac084
@morozov
Merge pull request doctrine#4438 from trusek/2.12.x
5a7befb 
doctrine#4437 MSSQL adds a redundant ORDER BY clause when with subquery with …
@greeflas @morozov
Fix and improve docs
a0b82ff
@morozov
Merge pull request doctrine#4260 from greeflas/fix_docs
373c24d 
Fix and improve docs
@nicwortel
Use the ramsey/composer-install action to install dependencies
4fb1862 
Follow-up of doctrine/.github#16.
@greg0ire
Merge pull request doctrine#4463 from nicwortel/ramsey-composer-install
4211e3c 
Use the ramsey/composer-install action to install dependencies
@morozov
Update PHPUnit to 9.5
f5984e1
@morozov
Merge pull request doctrine#4466 from morozov/phpunit-9.5
5ffd105 
Update PHPUnit to 9.5
@nicwortel
Remove composer.lock from version control
b40e327 
See https://github.com/orgs/doctrine/projects/7#card-50248076.
@nicwortel
Require the exact versions of development tools
02fbb3d 
This prevents randomly failing builds in case one of the dependencies releases
an unintentional BC break.
See https://github.com/doctrine/dbal/pull/4464/files#r542180416 for more
information.
@greg0ire
Merge pull request doctrine#4464 from nicwortel/remove-composer-lock
ce3421e 
Remove composer.lock from version control
greg0ire
greg0ire commented Jan 7, 2021
View reviewed changes
composer.json Outdated
"symfony/console": "^2.0.5|^3.0|^4.0|^5.0",
"vimeo/psalm": "^4.1"
"vimeo/psalm": "4.2.1"
Copy link
Member Author

Choose a reason for hiding this comment

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

@nicwortel why did you use 4.3.1 on your PR to 2.12.x? 2.12.x was using 4.1.1, and 3.0.x is using 4.2.1. Using 4.3.1 here results in some errors.

An interesting consequence of removing composer.lock is that now we can see discrepancies in tools version between branches.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right... my bad 🤦

I could have sworn that I used the exact version as it was locked by the composer.lock, but I must have messed up switching between versions at some point.

It seems like I accidentally also moved doctrine/coding-standard from 8.1.0 to 8.2.0.

Copy link
Member Author

@greg0ire greg0ire Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No worries, I'll fix the issues! Since we were using different versions for Psalm it's better anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I'm going to be forced to upgrade to 4.3.2 because we use strtok() with one argument in the codebase, and 4.3.1 takes issue with that (vimeo/psalm#4874)

@greg0ire greg0ire requested a review from morozov January 7, 2021 20:14
@greg0ire
Copy link
Member Author

greg0ire commented Jan 7, 2021

Looks like I messed up somewhere, there is a failure that seems related to #4437, @trusek can you maybe help? What I did is I removed the following block, because I think SQLServer2012Platform is the only SQL Server platform on this branch.

        if (
            $platform instanceof SQLServerPlatform
            && ! $platform instanceof SQLServer2012Platform
        ) {
            $subquery = preg_replace('/^SELECT\s/i', 'SELECT TOP 1 ', $subquery);
        }

@trusek
Copy link
Contributor

trusek commented Jan 7, 2021

Hey @greg0ire it looks like changes to SQLServer2012Platform from this commit have not been applied.

And If the SqlServer 2012 version is the latest supported version, this mentioned block of code can be safely removed

@greg0ire
Copy link
Member Author

greg0ire commented Jan 7, 2021

Thanks @trusek ! I don't know how I managed this one 😅

@greg0ire greg0ire merged commit db6f73c into doctrine:3.0.x Jan 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nicwortel nicwortel nicwortel left review comments

@morozov morozov morozov approved these changes

Assignees
No one assigned
Labels
Merge Up
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@greg0ire @trusek @morozov @nicwortel @kishore-hariram @greeflas