-
Notifications
You must be signed in to change notification settings - Fork 355
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
Update randomElements
to return random number of elements when no count is provided
#658
Conversation
07e73e4
to
de7cc0b
Compare
Unfortunately changing the default value of a parameter is a breaking change if it changes the output of the function. |
I understand, can we merge to the next major version though? I don't think it really makes sense to have to explicitly pass Or I suppose I could change the default back to |
Indeed, I think allowing Please be sure to include a test that asserts a count, which should work fine since we are seeding the Also please open a PR to the docs repository. |
5c4c973
to
a4a62f9
Compare
@bram-pkg done, is there a planned |
…ount is provided
Not as of yet. We're struggling to be motivated I think, outside our day jobs :) |
@bram-pkg can the community help with this? |
That's a good idea, it would be helpful if a 'Roadmap to 2.0' issue was created with what needs to be done and then the community could pitch in |
Will work on that |
Hello!
What is the reason for this PR?
This updates
randomElements
to return a random number of elements when no count is provided. Defaulting to a count of 1 is undesirable because if I only wanted one element I would just userandomElement
. This cleans up having to passfake()->numberBetween(1, count($array))
as the$count
when I don't care how many values are returned.Author's checklist
Summary of changes
$count
inBase::randomElement
nullable, and if null, then default to random number of elements.Review checklist