Skip to content

Conversation

amirhshokri
Copy link
Contributor

This PR refactors all instances of $this->orderBy(..., 'desc') to use the more expressive $this->orderByDesc(...) method from query builder.

@amirhshokri amirhshokri marked this pull request as draft October 11, 2025 18:29
@shaedrich
Copy link
Contributor

How long until @browner12 sends a PR reverting your changes (see #53563)?

@amirhshokri amirhshokri marked this pull request as ready for review October 11, 2025 18:39
@amirhshokri
Copy link
Contributor Author

I didn’t get it. Why would they revert it?

@shaedrich

@browner12
Copy link
Contributor

yah, don't do this. you growing the call stack and introducing a (minor) performance hit for no gain.

@amirhshokri
Copy link
Contributor Author

Makes sense. But then why is this method in the source code?

@browner12

@shaedrich
Copy link
Contributor

Because not everybody is a performance purist to the extent, browner is. And Laravel has a long history as a framework of offering convenience functions and methods.

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

@amirhshokri amirhshokri deleted the 12.x-use-order-by-desc branch October 12, 2025 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants