Clean phpdoc and arg names for relation traits #2587
Conversation
060293e to
47c4383
Compare
| * @param string $foreignKey | ||
| * @param string $relation | ||
| * @return \MongoDB\Laravel\Relations\EmbedsMany | ||
| * @param class-string $related |
There was a problem hiding this comment.
| use Illuminate\Database\Eloquent\Relations\MorphOne; | ||
| use Illuminate\Support\Str; | ||
| use MongoDB\Laravel\Eloquent\Model as MongoDBModel; |
There was a problem hiding this comment.
Was this necessary? We're not referencing another Model class in this file and I don't recall seeing an alias used elsewhere (in previous PRs).
There was a problem hiding this comment.
My idea was that it's more explicit that we need a MongoDB Model and not the Eloquent Base Model.
There was a problem hiding this comment.
Got it. No argument if this was done for readability 👍
| * @param string $relation | ||
| * @param class-string $related | ||
| * @param string|null $foreignKey | ||
| * @param string|null $ownerKey |
There was a problem hiding this comment.
Is the variable change here done for consistency with a base Laravel class/trait?
There was a problem hiding this comment.
Yes and for correctness.
| * @param class-string $related | ||
| * @param string|null $collection | ||
| * @param string|null $foreignPivotKey | ||
| * @param string|null $relatedPivotKey |
There was a problem hiding this comment.
$relatedPivotKey sounds much more descriptive than $otherKey. 👍
There was a problem hiding this comment.
That was the first motivation. I didn't understand the meaning of all this parameters.
Apply arg name change from laravel/framework#18380 and param types from laravel/framework#31252