-
Notifications
You must be signed in to change notification settings - Fork 32
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
Use VariableNode on "settings" configuration #370
Use VariableNode on "settings" configuration #370
Conversation
e35b4d5
to
492623f
Compare
note: in future we'll switch to fully defined settings structure |
7e6113e
to
4577461
Compare
"arrayNode" does not support arbitrary configuration keys. Therefore, I switch to variableNode with dynamic check. Fixed meilisearch#369
4577461
to
7aa5aac
Compare
@norkunas Seems like it generates a ConfigBuilder that allow passing arbitrary array. Sadly, I cannot make it work locally. It might not the fault of my code in this PR; maybe I need some additional code to make it registered? |
I'm not sure how to test that in a bundle |
https://symfony.com/doc/current/configuration.html#using-php-configbuilders; however, I afraid the types are generated only on the consumer. I don't really find a way to test it just in this bundle. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #370 +/- ##
============================================
+ Coverage 88.69% 88.73% +0.03%
Complexity 1 1
============================================
Files 20 20
Lines 911 914 +3
============================================
+ Hits 808 811 +3
Misses 103 103 ☔ View full report in Codecov by Sentry. |
5eda9ab
to
43dc7b3
Compare
43dc7b3
to
9fafbff
Compare
59c728f
to
acf34e4
Compare
bors merge |
Thank you @pan93412 |
371: Fix: Container extension "meili_search" is not registered. r=norkunas a=pan93412 # Pull Request ## Related issue #370 (comment) ## What does this PR do? "meili_search" does not match the bundle name, "meilisearch," which causes some strange bugs, such as this issue. I have completed a basic smoke test of PHP and YAML. I can confirm that the PHP configuration is now working with the Meilisearch bundle, while the old YAML configuration should function as it did before. ```bash $ php bin/console meilisearch:import --update-settings Importing for index App\Entity\Question Indexed a batch of 70 / 70 App\Entity\Question entities into dev_questions index (70 indexed since start) Setting "filterableAttributes" updated of "dev_questions". Setting "sortableAttributes" updated of "dev_questions". Done! ``` ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: Yi-Jyun Pan <[email protected]>
Pull Request
Related issue
Fixes #369
What does this PR do?
"arrayNode" does not support arbitrary configuration keys. Therefore, I switch to variableNode with dynamic check.
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!