refactor(tool/parameters): update NewParameter functions to use functional options pattern#3314
Merged
Yuan325 merged 4 commits intoJun 23, 2026
Conversation
Deeven-Seru
force-pushed
the
refactor/functional-options-parameters
branch
from
b52ea02 to
dc748e9
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors parameter initialization across all database tools to use a unified functional options pattern instead of specialized constructor functions. Feedback on these changes highlights a printf-style formatting bug in the oraclesql tool's DebugContext call, a discrepancy between the default value and description in the postgreslistroles tool, and numerous instances where the automated migration left awkward blank lines and broken indentation that should be cleaned up for readability.
Deeven-Seru
force-pushed
the
refactor/functional-options-parameters
branch
from
dc748e9 to
3526a48
Compare
Yuan325
requested changes
Jun 17, 2026
Contributor
There was a problem hiding this comment.
Hi @Deeven-Seru Thanks for submitting this PR! :) Some feedback regarding cleaning up duplication code and extra lines~ I'll take another look once updated. THank you again!
Please also resolve the merge conflicts, thank you!
Yuan325
added
the
release candidate
Deeven-Seru
force-pushed
the
refactor/functional-options-parameters
branch
from
3526a48 to
367f159
Compare
Contributor
Author
|
hi @Yuan325 I've addressed the feedback :) |
Yuan325
added
the
priority: p1
Yuan325
requested changes
Jun 23, 2026
Yuan325
left a comment
Yuan325
left a comment
Contributor
There was a problem hiding this comment.
Last feedback, can you please clean up the extra lines added in some files? Will approve once updated, thankyou again!!!
…ional options pattern
Deeven-Seru
force-pushed
the
refactor/functional-options-parameters
branch
from
4a3d885 to
c78cca5
Compare
Contributor
Author
|
hi @Yuan325 yuan Ive addressed the feedback , thanks for reviewing :) |
Yuan325
reviewed
Jun 23, 2026
Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
Yuan325
approved these changes
Jun 23, 2026
Contributor
|
Thank you @Deeven-Seru ! :) I'll go ahead and merge this. |
Contributor
|
/gcbrun |
Contributor
|
/gcbrun |
5 tasks
github-actions Bot
pushed a commit
to Deeven-Seru/mcp-toolbox
that referenced
this pull request
Jun 24, 2026
…se functional options pattern (googleapis#3314) ## Description This PR refactors all `New*Parameter` constructor functions in `internal/util/parameters/parameters.go` to use the **functional options pattern**, replacing the proliferation of `NewXParameterWithOption(...)` variants. The old design required a separate constructor for every combination of options (e.g. `NewStringParameterWithDefault`, `NewStringParameterWithRequired`, `NewStringParameterWithAuth`, etc.). This led to a combinatorial explosion of function signatures and made it impossible to combine options without adding yet another constructor. The new design introduces: - A `*ParameterOption` functional type per parameter kind - `With*` option constructors (e.g. `WithStringRequired`, `WithStringDefault`, `WithStringAllowedValues`) - A single `New*Parameter(name, desc, ...opts)` constructor per type This makes it trivial to freely combine any options: ```go // Before — rigid, one constructor per option combo parameters.NewStringParameterWithRequired("name", "desc", true) // After — freely composable parameters.NewStringParameter("name", "desc", parameters.WithStringRequired(true), parameters.WithStringAllowedValues([]any{"a", "b", "c"}), ) ``` ### Changes - Defined `*ParameterOption` functional types and `With*` functions for all 6 parameter types (`String`, `Int`, `Float`, `Boolean`, `Array`, `Map`) - Updated all `New*Parameter` constructors to accept variadic `opts ...Option` - Migrated **600+ call sites** across `internal/` and `tests/` using `gofmt -r` rewrite rules - Removed all obsolete `NewXParameterWithOption` functions - Added `WithIntMinValue`, `WithIntMaxValue`, `WithFloatMinValue`, `WithFloatMaxValue` range options ## PR Checklist - [x] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involves a breaking change ## Issue Reference Fixes googleapis#3311 🦕 --------- Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 2cca43e
Contributor
Author
|
thanks for the review @Yuan325 :) |
github-actions Bot
pushed a commit
to cabletequernd/genai-toolbox-slim
that referenced
this pull request
Jun 24, 2026
…se functional options pattern (googleapis#3314) ## Description This PR refactors all `New*Parameter` constructor functions in `internal/util/parameters/parameters.go` to use the **functional options pattern**, replacing the proliferation of `NewXParameterWithOption(...)` variants. The old design required a separate constructor for every combination of options (e.g. `NewStringParameterWithDefault`, `NewStringParameterWithRequired`, `NewStringParameterWithAuth`, etc.). This led to a combinatorial explosion of function signatures and made it impossible to combine options without adding yet another constructor. The new design introduces: - A `*ParameterOption` functional type per parameter kind - `With*` option constructors (e.g. `WithStringRequired`, `WithStringDefault`, `WithStringAllowedValues`) - A single `New*Parameter(name, desc, ...opts)` constructor per type This makes it trivial to freely combine any options: ```go // Before — rigid, one constructor per option combo parameters.NewStringParameterWithRequired("name", "desc", true) // After — freely composable parameters.NewStringParameter("name", "desc", parameters.WithStringRequired(true), parameters.WithStringAllowedValues([]any{"a", "b", "c"}), ) ``` ### Changes - Defined `*ParameterOption` functional types and `With*` functions for all 6 parameter types (`String`, `Int`, `Float`, `Boolean`, `Array`, `Map`) - Updated all `New*Parameter` constructors to accept variadic `opts ...Option` - Migrated **600+ call sites** across `internal/` and `tests/` using `gofmt -r` rewrite rules - Removed all obsolete `NewXParameterWithOption` functions - Added `WithIntMinValue`, `WithIntMaxValue`, `WithFloatMinValue`, `WithFloatMaxValue` range options ## PR Checklist - [x] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involves a breaking change ## Issue Reference Fixes googleapis#3311 🦕 --------- Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 2cca43e
5 tasks
duwenxin99
added a commit
that referenced
this pull request
Jun 30, 2026
## 1. Description Completes configurable parameter support for the remaining Cloud Storage object tools, using the functional-option parameter constructors introduced in #3314. This change adds optional config-backed values for download, upload, write, copy, move, and delete object tools. Configured fields are hidden from the runtime manifest while zero-config tools keep their current parameter schemas. Download also supports a configured `destination_dir`, where the runtime `destination` remains visible but becomes a relative path constrained under that directory. ## 2. PR Checklist - [x] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involves a breaking change ## 3. Issue Reference Related to #3314 🦕 Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 30, 2026
…rs (#3529) ## 1. Description Completes configurable parameter support for the remaining Cloud Storage object tools, using the functional-option parameter constructors introduced in #3314. This change adds optional config-backed values for download, upload, write, copy, move, and delete object tools. Configured fields are hidden from the runtime manifest while zero-config tools keep their current parameter schemas. Download also supports a configured `destination_dir`, where the runtime `destination` remains visible but becomes a relative path constrained under that directory. ## 2. PR Checklist - [x] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involves a breaking change ## 3. Issue Reference Related to #3314 🦕 Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com> d6dc5fe
github-actions Bot
pushed a commit
to rodineyw/mcp-toolbox
that referenced
this pull request
Jun 30, 2026
…rs (googleapis#3529) ## 1. Description Completes configurable parameter support for the remaining Cloud Storage object tools, using the functional-option parameter constructors introduced in googleapis#3314. This change adds optional config-backed values for download, upload, write, copy, move, and delete object tools. Configured fields are hidden from the runtime manifest while zero-config tools keep their current parameter schemas. Download also supports a configured `destination_dir`, where the runtime `destination` remains visible but becomes a relative path constrained under that directory. ## 2. PR Checklist - [x] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involves a breaking change ## 3. Issue Reference Related to googleapis#3314 🦕 Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com> d6dc5fe
github-actions Bot
pushed a commit
to Jaleel-zhu/genai-toolbox
that referenced
this pull request
Jun 30, 2026
…rs (googleapis#3529) ## 1. Description Completes configurable parameter support for the remaining Cloud Storage object tools, using the functional-option parameter constructors introduced in googleapis#3314. This change adds optional config-backed values for download, upload, write, copy, move, and delete object tools. Configured fields are hidden from the runtime manifest while zero-config tools keep their current parameter schemas. Download also supports a configured `destination_dir`, where the runtime `destination` remains visible but becomes a relative path constrained under that directory. ## 2. PR Checklist - [x] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involves a breaking change ## 3. Issue Reference Related to googleapis#3314 🦕 Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com> d6dc5fe
github-actions Bot
pushed a commit
to pepe57/genai-toolbox
that referenced
this pull request
Jun 30, 2026
…rs (googleapis#3529) ## 1. Description Completes configurable parameter support for the remaining Cloud Storage object tools, using the functional-option parameter constructors introduced in googleapis#3314. This change adds optional config-backed values for download, upload, write, copy, move, and delete object tools. Configured fields are hidden from the runtime manifest while zero-config tools keep their current parameter schemas. Download also supports a configured `destination_dir`, where the runtime `destination` remains visible but becomes a relative path constrained under that directory. ## 2. PR Checklist - [x] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involves a breaking change ## 3. Issue Reference Related to googleapis#3314 🦕 Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com> d6dc5fe
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
3 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.
Description
This PR refactors all
New*Parameterconstructor functions ininternal/util/parameters/parameters.goto use the functional options pattern, replacing the proliferation ofNewXParameterWithOption(...)variants.The old design required a separate constructor for every combination of options (e.g.
NewStringParameterWithDefault,NewStringParameterWithRequired,NewStringParameterWithAuth, etc.). This led to a combinatorial explosion of function signatures and made it impossible to combine options without adding yet another constructor.The new design introduces:
*ParameterOptionfunctional type per parameter kindWith*option constructors (e.g.WithStringRequired,WithStringDefault,WithStringAllowedValues)New*Parameter(name, desc, ...opts)constructor per typeThis makes it trivial to freely combine any options:
Changes
*ParameterOptionfunctional types andWith*functions for all 6 parameter types (String,Int,Float,Boolean,Array,Map)New*Parameterconstructors to accept variadicopts ...Optioninternal/andtests/usinggofmt -rrewrite rulesNewXParameterWithOptionfunctionsWithIntMinValue,WithIntMaxValue,WithFloatMinValue,WithFloatMaxValuerange optionsPR Checklist
!if this involves a breaking changeIssue Reference
Fixes #3311 🦕