-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PHPLIB-1305 Support "codec" option in listSearchIndexes() #1190
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue has no fixVersion, but I have no qualms about throwing it into master
before we branch v1.17.
The codec is a new feature for v1.17. I don't see what alternative there would be to merging it into master before the 1.17.0 release. |
I wasn't sure if this was something we intended to fix in 1.17 or defer to 1.18. If 1.18, then it would only be merged to master after branching v1.17. |
I'm fine tossing it into 1.17 - however there should be at least a single test. For all I care, it could be a test passing an invalid codec, expecting an |
@@ -28,6 +28,8 @@ public function provideInvalidConstructorOptions(): array | |||
$options[][] = ['batchSize' => $value]; | |||
} | |||
|
|||
$options[][] = ['codec' => 'foo']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alcaeus test added.
Fix PHPLIB-1305
It's definitely not a place where I would expect a codec to be used, but why not.
I don't think we need to test all the options.