chore: refactor add_extension to reduce line count #6278
Conversation
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
Extract helper functions from the 259-line add_extension function:
- merge_environments: async function to merge env vars from direct envs and keychain
- substitute_env_vars: function to substitute env vars in strings (supports ${VAR} and $VAR)
- create_streamable_http_client: async function to create StreamableHttp MCP clients
- create_stdio_client: async function to create Stdio MCP clients
The add_extension function is now 129 lines (down from 259), well under the 200 line target.
Also updated the test to use the module-level substitute_env_vars function instead of duplicating it.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR successfully refactors the add_extension function in extension_manager.rs to reduce its line count from 259 to 129 lines by extracting helper functions. The refactoring is purely structural with no behavioral changes.
- Extracts four helper functions:
merge_environments,substitute_env_vars,create_streamable_http_client, andcreate_stdio_client - Updates the test to use the newly module-level
substitute_env_varsfunction instead of duplicating it - Maintains all existing functionality and error handling patterns
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Refactors the
add_extensionfunction inextension_manager.rsto reduce its line count from 259 lines to 129 lines (well under the 200 line target).Changes
Extracted the following helper functions from
add_extension:merge_environments: Async function to merge environment variables from direct envs and keychain-stored env_keyssubstitute_env_vars: Function to substitute environment variables in strings (supports both ${VAR} and $VAR syntax)create_streamable_http_client: Async function to create StreamableHttp MCP clients with header substitution and OAuth supportcreate_stdio_client: Async function to create Stdio MCP clients with malware checkingAlso updated the test to use the module-level
substitute_env_varsfunction instead of duplicating it.Testing
Related
Part of TSK-696: Simplify clippy baseline system