Skip to content
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

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Nov 8, 2023

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.

@GromNaN GromNaN requested review from jmikola and alcaeus November 8, 2023 15:33
Copy link
Member

@jmikola jmikola left a 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.

@GromNaN
Copy link
Member Author

GromNaN commented Nov 8, 2023

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.

@jmikola
Copy link
Member

jmikola commented Nov 9, 2023

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.

@alcaeus
Copy link
Member

alcaeus commented Nov 9, 2023

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 InvalidArgumentException to ensure that the codec option is handled somewhere.

@@ -28,6 +28,8 @@ public function provideInvalidConstructorOptions(): array
$options[][] = ['batchSize' => $value];
}

$options[][] = ['codec' => 'foo'];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alcaeus test added.

@alcaeus alcaeus merged commit a01f3cf into mongodb:master Nov 9, 2023
13 checks passed
@GromNaN GromNaN deleted the PHPLIB-1305 branch November 10, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants