Add vararg constructor to InMemoryEntityLookup #205
Conversation
| /** | ||
| * @param EntityDocument ...$entities | ||
| */ | ||
| public function __construct( ...$entities ) { |
There was a problem hiding this comment.
I'm somewhat surprised by this. I faced heavy backlash when I tried to introduce pretty much the exact same back then in wmde/WikibaseDataModel#376 and wmde/WikibaseDataModel#416. Not meant to block this, not even as a vote. I'm just curious what changed and what others think nowadays.
There was a problem hiding this comment.
What I am doing here is adding an array parameter using varargs. That is not the same as changing an existing parameter to support two input styles. The context is also different.
There was a problem hiding this comment.
I would not call this an "array parameter". The ... syntax as documented at http://php.net/manual/en/functions.arguments.php#functions.variable-arg-list does not allow callers to pass an array, but a "variable-length list of arguments". Which is what I wanted in these other pull requests.
JeroenDeDauw
force-pushed
the
InMemoryEntityLookup
branch
from
ff7819a
to
660db6d
Compare
JeroenDeDauw
requested review from
addshore,
manicki,
mariushoch,
tarrow and
lucaswerkmeister
|
@manicki can you get this simple change unstuck? |
JeroenDeDauw
force-pushed
the
InMemoryEntityLookup
branch
from
bd7d8e2
to
9164a05
Compare
|
+1 Looks good to me |
Noticed absence in
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseQualityConstraints/+/473719/1/tests/phpunit/Helper/ExceptionIgnoringEntityLookupTest.php