- Issue created by @DrDam
- First commit to issue fork.
- 🇨🇦Canada mparker17 UTC-4
@drdam, thank you for the contribution!
I committed your code to a merge request (and updated the issue summary) to make it easier for me to review.
I changed the version to
8.0.x-dev
because this issue applies to the latest development version as well.I took a very quick look at your patch: it looks good to me (I anticipate some phpcs notices about documentation, but I expect that will be easy to fix).
Thanks for tagging this with "Needs tests"! Is there anything that I can do to help with writing tests for this change?
- 🇨🇦Canada mparker17 UTC-4
I notice that Testbot is detecting a failing test.
Fortunately, I think it should be pretty easy to resolve: the code that constructs a
new QueryParamsEvent
in\Drupal\Tests\elasticsearch_connector\Unit\SearchAPI\Query\QueryParamBuilderTest::testBuildQueryParams()
will need to be updated to pass aQuery
(or a test double for aQuery
). - 🇨🇦Canada mparker17 UTC-4
Also, we should add or update a change record to assist people upgrading from 8.x-7.x. Note we have a draft, not-complete-yet change record with some information on events that existed in the 8.x-7.x and their equivalents in 8.0.x (if applicable) → — not sure if we need to update that, or create a new change record.
- 🇨🇦Canada mparker17 UTC-4
(sorry for the noise: I rebased onto the latest
8.0.x
, so that the phpstan pipeline wouldn't warn us about a deprecation that was fixed in8.0.x
) - 🇨🇦Canada mparker17 UTC-4
(sorry for the noise: I rebased onto the latest
8.0.x
, so that the phpstan pipeline wouldn't warn us about a deprecation that was fixed in8.0.x
)