[11.x] convert collect() helper to new Collection()
#53726
Conversation
|
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
i screwed up when I did the merge
|
How about converting also the framework/src/Illuminate/Collections/Traits/EnumeratesValues.php Lines 114 to 117 in a57d4ba |
|
If this is actually needed, this should be done using a custom PHP CS Fixer rule as that would also enforce it going forward. I certainly would just do Personally I think framework itself should take advantage of all the helpers as a proving ground, unless there's a serious performance hit in something like loops. |
|
@chu121su12 I can try that in a separate PR. @tontonsb I would love to add automation for this. Any thoughts on how to get started? |
|
@browner12 sorry, it looks harder than I assumed. The |
while there is no behavior change in this commit, my intent behind this is to help prevent stupid mistakes from assuming what Collection is being referenced. by always aliasing this class explicitly, we gain a little confidence when in the code about what version we are referencing. our use of `Illuminate\Support\Collection` far outweights our use of `Illuminate\Database\Eloquent\Collection`. however, it's very common for both to be used in a file that uses `Illuminate\Database\Eloquent\Collection`. therefore, I say we treat the base Collection as the default, and our Eloquent Collection as our alias ALL the time. this will provide consistency and help avoid stupid mistakes. it is also helpful when doing global searches because now we have a unique token to search for. this idea came to me as I was working on laravel#53726 because making sure I was referencing the correct Collection was the gotcha I had to pay the closest attention to.
while there is no behavior change in this commit, my intent behind this is to help prevent stupid mistakes from assuming what Collection is being referenced. by always aliasing this class explicitly, we gain a little confidence when in the code about what version we are referencing. our use of `Illuminate\Support\Collection` far outweights our use of `Illuminate\Database\Eloquent\Collection`. however, it's very common for both to be used in a file that uses `Illuminate\Database\Eloquent\Collection`. therefore, I say we treat the base Collection as the default, and our Eloquent Collection as our alias ALL the time. this will provide consistency and help avoid stupid mistakes. it is also helpful when doing global searches because now we have a unique token to search for. this idea came to me as I was working on #53726 because making sure I was referencing the correct Collection was the gotcha I had to pay the closest attention to.
4 participants
continuation of #53563