Create shared helper module for tsp-client commands#12051
Closed
samvaity wants to merge 13 commits into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the TypeSpec conversion tool to use a shared helper module for tsp-client commands. The goal is to create reusable logic that can be shared across multiple tools while maintaining loose coupling between UI commands and core business logic.
- Extracts the
tsp-client convertcommand execution logic into a dedicated helper class - Introduces dependency injection for the new
ITspClientHelperinterface - Updates existing tests to accommodate the new constructor dependencies
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TspConvertTool.cs | Refactored to use injected ITspClientHelper instead of inline tsp-client execution |
| ServiceRegistrations.cs | Added DI registration for the new ITspClientHelper interface |
| TspClientHelper.cs | New implementation class containing the extracted tsp-client convert logic |
| ITspClientHelper.cs | New interface defining the contract for tsp-client operations |
| TspConvertToolTest.cs | Updated test constructor calls to include the new dependency |
Comments suppressed due to low confidence (1)
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/TspConvertTool.cs:161
- Duplicate file existence check. Lines 151-155 already perform the same validation with
fullPathvariable. Remove this redundant check.
var fullPathToSwaggerReadme = Path.GetFullPath(pathToSwaggerReadme.Trim());
if (!File.Exists(fullPathToSwaggerReadme))
{
return $"Failed: pathToSwaggerReadme '{fullPathToSwaggerReadme}' does not exist.";
}
* Add variable updates on the yaml * Add variable updates on the yaml * as variable and with condition * variables * debugging * debugging * lets try * initial approach again? * Simplified * Fix comparison to string * Clean up codE
* update logic * up --------- Co-authored-by: Kyle Zhang <v-zhanh@microsoft.com>
…iling, and update `GetRepoRemoteUri` to support ssh URIs by converting to https (#12036) Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
* [Python APIView] add quotes to string Literals * changelog * apply code review comments * change order of azure tests to fix random tox error * fix test * fix test
* Refactor evals. Add `cli eval run` command. * Add tiny test for debugging. * Refactor to avoid importing evals in CLI. * Temporary testing. * Eval fixes. Should be runnable! * Clean up. * Fixes. * Fixes.
Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
Member
Author
|
Did a poor rebase |
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
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.
Created shared helpers for tsp-client commands (common utility class used by multiple tools) so they provide single implementation of core logic