laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.62k stars 11.03k forks source link

[11.x] Removing unused var assignment in Illuminate Router #53575

Closed Carnicero90 closed 2 days ago

Carnicero90 commented 2 days ago

It won't benefit any end user, it can't break any existing feature since the variable was unused, it doesn't make building applications easier: I just noticed that while debugging some stuff in the vendor of my application.

osbre commented 2 days ago

@Carnicero90 It was meant to help with readability. I think it came from the days when there were no named arguments in PHP. nowadays it should be:

$route->setAction($this->mergeWithLastGroup(
    $route->getAction(),
    prependExistingPrefix: false,
));
Carnicero90 commented 2 days ago

Aaah, I see. Here you go.

rodrigopedra commented 2 days ago

Named arguments are not covered by Laravel's backwards compatibility guidelines. We may choose to rename function arguments when necessary in order to improve the Laravel codebase. Therefore, using named arguments when calling Laravel methods should be done cautiously and with the understanding that the parameter names may change in the future.

Reference: https://laravel.com/docs/11.x/releases#named-arguments

Carnicero90 commented 2 days ago

Named arguments are not covered by Laravel's backwards compatibility guidelines. We may choose to rename function arguments when necessary in order to improve the Laravel codebase. Therefore, using named arguments when calling Laravel methods should be done cautiously and with the understanding that the parameter names may change in the future.

Reference: https://laravel.com/docs/11.x/releases#named-arguments

Seems to be more of a user guide though. Still, it doesnt matter all that much, since this is a pretty useless pr overall.