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

[5.4] Support amount 0 in Arr::random() #20439

Merged
merged 9 commits into from
Aug 6, 2017
Merged

[5.4] Support amount 0 in Arr::random() #20439

merged 9 commits into from
Aug 6, 2017

Conversation

vlakoff
Copy link
Contributor

@vlakoff vlakoff commented Aug 5, 2017

Follow-up to #20396 and #20402.

To summarize:

  • Supports amount 0 in Arr::random() for consistency with Collection::random()
  • Supports "0" as a string, for consistency as the other numbers are accepted as strings.
  • A handful of additions to the tests, better this way to prevent any risk of regressions.


ping @browner12

@vlakoff vlakoff changed the title Random [5.4] Support amount 0 in Arr::random() Aug 5, 2017
vlakoff added 5 commits August 5, 2017 23:37
I overlooked it, so I guess it's better to consolidate in one method.
For consistency with Collection::random()
Now redundant because Arr::random() supports it.
DRY principle, and the case is covered by tests.
For consistency with other numbers, as they are accepted as strings.
@vlakoff
Copy link
Contributor Author

vlakoff commented Aug 6, 2017

Added a commit to remove the redundant "enough items available" validation code.

I was tired of seeing this duplication only because the exception messages were slightly different :)

@taylorotwell taylorotwell merged commit 5d55936 into laravel:5.4 Aug 6, 2017
@vlakoff vlakoff deleted the random branch August 7, 2017 01:06
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.

2 participants