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

tooManyAttempts method of RateLimiter class parameter fix #22197

Closed
wants to merge 4 commits into from

Conversation

khanrn
Copy link
Contributor

@khanrn khanrn commented Nov 24, 2017

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

themsaid and others added 4 commits November 23, 2017 08:09
* Update docs for queue:retry command

At the moment its not obvious that you can pass 'all'
as an id without looking through the code.

* Update RetryCommand.php
* Fix connection resolver and static side effect

Fixes laravel#22133

Signed-off-by: Beau Hastings <beausy@gmail.com>

* Formatting
@jmarcher
Copy link
Contributor

This should be pointing to master

@khanrn khanrn changed the base branch from 5.5 to master November 24, 2017 13:00
@khanrn
Copy link
Contributor Author

khanrn commented Nov 24, 2017

@jmarcher Pointed to master branch. Changed from 5.5. What about #22190 and #22191 ?

@jmarcher
Copy link
Contributor

Those are fine in this case you are modifying the public API that's why.

And btw, you need to rebase your branch

@sisve
Copy link
Contributor

sisve commented Nov 24, 2017

What problem does this change actually fix?

@taylorotwell
Copy link
Member

Has a bunch of other commits in the history. Please resubmit fresh to master.

@khanrn
Copy link
Contributor Author

khanrn commented Nov 24, 2017

@jmarcher Thanks for clarifying the API thing. And I just changed the pointing branch in GitHub web UI form 5.5 to master. That's why this rebase issue occurred.

@sisve This $decayMinutes = 1 parameter has no usage inside the method. Why should we put a parameter there which has no usage in the method ? BTW, my new submission is #22202. We can continue this discussion there.

@taylorotwell My new submission is #22202 . Please have a look when you get time.

@khanrn khanrn deleted the rate-limiter-class-method-fix branch November 24, 2017 16:21
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.

None yet

7 participants