[AI Bundle] Use stringNode() and integerNode() for string configuration values#589
Closed
OskarStark wants to merge 0 commit intosymfony:mainfrom
Closed
[AI Bundle] Use stringNode() and integerNode() for string configuration values#589OskarStark wants to merge 0 commit intosymfony:mainfrom
stringNode() and integerNode() for string configuration values#589OskarStark wants to merge 0 commit intosymfony:mainfrom
Conversation
carsonbot
changed the title
stringNode() for string configuration valuesstringNode() for string configuration values
OskarStark
changed the title
stringNode() for string configuration values
GromNaN
approved these changes
Sep 15, 2025
OskarStark
changed the title
stringNode() and integerNode() for string configuration values
Contributor
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Might be wrong but I feel like a lot of remaining scalarNode could be stringNode
VincentLanglet
approved these changes
Sep 15, 2025
OskarStark
force-pushed
the
string-node
branch
2 times, most recently
from
9cc1570 to
9c95b45
Compare
OskarStark
added a commit
that referenced
this pull request
Sep 17, 2025
…tring configuration values (OskarStark) This PR was squashed before being merged into the main branch. Discussion ---------- [AI Bundle] Use `stringNode()` and `integerNode()` for string configuration values | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Docs? | no | Issues | -- | License | MIT Replace `scalarNode()` with more specific node types for better configuration type validation: - Use `stringNode()` for string values (API keys, URLs, service names, etc.) - Use `integerNode()` for integer values (dimensions, etc.) - Keep `scalarNode()` only for mixed-type fields where appropriate This provides better type validation and makes the configuration more self-documenting. ## Changes Made - **String fields**: API keys, URLs, endpoints, service names, class names, database/collection names, etc. now use `stringNode()` - **Integer fields**: `dimensions` and `top_k` fields now use `integerNode()` - **Mixed fields**: Strategy, metric, and similar fields remain as `scalarNode()` ## Needs - [ ] #277 Commits ------- 492f75f [AI Bundle] Use `stringNode()` and `integerNode()` for string configuration values
OskarStark
force-pushed
the
string-node
branch
from
119572d to
492f75f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace
scalarNode()with more specific node types for better configuration type validation:stringNode()for string values (API keys, URLs, service names, etc.)integerNode()for integer values (dimensions, etc.)scalarNode()only for mixed-type fields where appropriateThis provides better type validation and makes the configuration more self-documenting.
Changes Made
stringNode()dimensionsandtop_kfields now useintegerNode()scalarNode()Needs