Skip to content

Conversation

@AJenbo
Copy link
Contributor

@AJenbo AJenbo commented Nov 8, 2018

This provides the call back with a attempt count, this can be helpful for function that need to modify there behavior with each attempt, or log it some how. Existing code should be unaffected since there was not parameter earlier.

@BrandonSurowiec
Copy link
Contributor

BrandonSurowiec commented Nov 9, 2018

This method was written using goto for a very specific reason.

Check out this PR that contains links to threads about it: #15973

(Also, this PR would need tests.)

This provides the call back with a attempt count, this can be helpful
for function that need to modify there behavior with each attempt, or
log it some how.
@AJenbo
Copy link
Contributor Author

AJenbo commented Nov 9, 2018

Thanks for the quick review. I have rewritten the code so that it keeps the goto and added a test for retry that tests this and the existing features.

I would add that the goto feel like an micro-optimization (is it true for 7.2, will it be true for 8.0?), with the addition of $attempt++ this would also mean that it saves a jump or two at the cost of an additional $times--, also the php code is 7 lines longer.

@driesvints driesvints changed the title Provide retry callback with the attempt count [5.8] Provide retry callback with the attempt count Nov 9, 2018
@taylorotwell taylorotwell merged commit 96de5cc into laravel:master Nov 9, 2018
@AJenbo AJenbo deleted the patch-1 branch November 9, 2018 19:36
@AJenbo AJenbo restored the patch-1 branch November 9, 2018 19:36
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.

3 participants