[5.6] Parameter has no usage inside the method. So we better remove it.

[khanrn]

$decayMinutes = 1 parameter has no usage inside the method. So we better remove it.

[khanrn]

@m1guelpf I'm kinda curious about your down vote. Would you please explain it a little bit ? I'm very interested to learn. :)

[jmarcher]

Probably because this PR is incomplete, tooManyAttempts is used in a few places, eliminating this parameter would break those parts.

Check:

framework/src/Illuminate/Foundation/Auth/ThrottlesLogins.php

Line 22 in 8f31c83

return $this->limiter()->tooManyAttempts(

and

framework/src/Illuminate/Routing/Middleware/ThrottleRequests.php

Line 51 in 8f31c83

if ($this->limiter->tooManyAttempts($key, $maxAttempts, $decayMinutes)) {

[khanrn]

Well @jmarcher I don't think this will break anything. Cause passing more arguments than expected to a method is supported from PHP 4. So changing to tooManyAttempts methods parameter will not harm anything.

Beside, this $decayMinutes parameter is doing nothing inside tooManyAttempts method. So, if a parameter is doing nothing then how it can harm anything.

[jmarcher]

So you make a PR deleting one unused parameter but still want to leave unused parameters to this very same method calls?

[khanrn]

Uha.. I never said that @jmarcher. That's different thing and I'm working on that. My point is it wouldn't break anything. But better to clear it everywhere and that's what I'm doing right now. Anyway, thanks for pointing that out and make it clear.

[sisve]

We cannot remove the calls that sends in a $decayMinutes argument; that would be weird for everyone that has a custom RateLimiter that actually uses the parameter. Are we sure that this isn't a bug where we actually should be using $decayMinutes in the tooManyAttempts() method?

[laurencei]

Are we sure that this isn't a bug where we actually should be using $decayMinutes in the tooManyAttempts() method?

It was made redunant when @themsaid did this PR: #20759

So looks intentional.

[taylorotwell]

@themsaid can you review this?

[themsaid]

I originally left the parameter to avoid having a breaking change with people using a custom limiter that extends this one, the change was only 5 days before the release so it made sense back then.

For 5.6 I think it's safe to remove the parameter as it's not used in this method anymore.

[jmarcher]

I know this may slow down the Framework development speed but I think we should first Deprecate method/parameters/attributes in the first release and remove in the second one.

Like discussed here: laravel/ideas#383

This type of changes do take a lot of effort for package developers or people customizing the core for their needs.