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

Show Expr::countDistinct() and Expr::concat() use variable-length argument lists #9911

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

craigfrancis
Copy link
Contributor

Was PR #9909... somehow I committed to the 2.12.x branch, and have made a mess of the rebase.


The functions Expr::countDistinct() and Expr::concat() both use func_get_args() to accept a variable number of arguments.

They should explicitly show this via the "..." operator, which has been supported since PHP 5.6.

@greg0ire
Copy link
Member

somehow I committed to the 2.12.x branch, and have made a mess of the rebase.

Don't commit anything to numbered branches, and create one new branch per MR, it will be easier to understand 😉

@greg0ire greg0ire added this to the 3.0.0 milestone Jul 18, 2022
@greg0ire
Copy link
Member

I experience the same build failure as you do with DBAL 4 on my own PR #9912

The build is broken, can't merge anything until it gets fixed.

@craigfrancis
Copy link
Contributor Author

Yep, I normally do, but not been thinking today (currently 37°C)

@greg0ire
Copy link
Member

🥵

@greg0ire
Copy link
Member

Created #9913 for communication purposes

@greg0ire greg0ire closed this Jul 18, 2022
@greg0ire greg0ire reopened this Jul 18, 2022
@greg0ire greg0ire enabled auto-merge July 18, 2022 21:29
@greg0ire greg0ire merged commit 220bfa1 into doctrine:3.0.x Jul 18, 2022
@greg0ire
Copy link
Member

Thanks @craigfrancis !

@craigfrancis
Copy link
Contributor Author

Thanks Grégoire, and sorry for making such a mess of a simple PR :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants