Improve docblock for with() method to clarify it adds to existing eag… #54838
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…er loads The previous docblock for the with() method wasn't entirely clear on whether it set and replaced the existing eager loading list or appended to it. I had to read the source code instead of just the docblock. This update clarifies that calling with() adds to the existing list rather than overwriting it.
- * Set the relationships that should be eager loaded.
+ * Specify relationships that should be eager loaded.Am I missing something here? The docblock literally said "set"—how is "specify" more precise? oO |
|
I feel that "Set" implies replacing existing relationships, "Specify" is better. However, "Adds to" is the clearest option IMO |
|
Yeah, I agree with you on the perceived meaning of "set", but "specify" doesn't really have such an agreed upon meaning as far as I know. What about "add"? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous docblock for the with() method wasn't entirely clear on whether it set and replaced the existing eager loading list or appended to it. I had to read the source code instead of just the docblock.
This update clarifies that calling with() adds to the existing list rather than overwriting it.