[azsdk-cli] Organize command line hierarchy#12564
Conversation
benbp
added
the
Central-EngSys
benbp
added
the
azsdk-cli
ff228ea to
5857828
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR reorganizes the CLI command hierarchy to improve usability and consistency. The primary goal is to establish a clear, logical grouping of commands under well-defined top-level command groups, making the CLI easier to navigate and understand. Additionally, this work aligns with the documented command structure guidelines.
Key Changes:
- Introduced consistent command aliases (e.g.,
pkgforpackage,tspfortypespec,azpforpipeline) - Reorganized commands under logical hierarchies (e.g., TypeSpec commands under
tsp, config commands underconfig) - Renamed commands for clarity and brevity (e.g.,
convert-swagger→convert,customized-update→update) - Removed outdated spec documentation files
- Added validation logic to detect duplicate commands in DEBUG builds
Reviewed Changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
cli-commands-guidelines.md |
Updated command hierarchy documentation to reflect new structure with aliases |
SharedCommandGroups.cs |
Added new command groups (Config, PackageReadme, PackageTest, TypeSpecProject, TypeSpecClient) and aliases for existing groups |
CommandGroup.cs |
Extended record to support command aliases |
CommandRunner.cs |
Added alias registration logic and DEBUG-mode duplicate command validation |
HostServerCommand.cs |
Renamed start command to mcp with start as hidden legacy alias |
| Various Tool files | Updated command hierarchies and renamed commands for consistency |
| Test files | Updated references to renamed classes and removed obsolete tests |
| Spec files | Removed outdated specification template and example files |
| Java test cleanup | Removed tests for deprecated snippet and javadoc functionality |
5857828 to
220021f
Compare
| ) : MCPMultiCommandTool | ||
| ) : MCPNoCommandTool | ||
| { | ||
| public override CommandGroup[] CommandHierarchy { get; set; } = [new("spec-pr", "Pull request tools")]; |
There was a problem hiding this comment.
FYI @praveenkuttappan @danieljurek I just removed the PR helpers from CLI mode, they are pretty redundant with the github CLI.
| ]; | ||
|
|
||
| private const string RunChecksCommandName = "run-checks"; | ||
| private readonly Argument<PackageCheckType> checkTypeArg = new("check-type") |
There was a problem hiding this comment.
FYI @l0lawrence I moved the check type to arguments instead an enum iteration to simplify the GetCommands() implementation here (this might allow me to refactor it in the base class to be a property instead of a method). The command line will still allow a lowercase check type and default to all however the help text shows an uppercase check type. I'm going to see if I can fix that so it always shows lowercase.
| ); | ||
|
|
||
| public static readonly CommandGroup PackageTest = new( | ||
| Verb: "test", |
There was a problem hiding this comment.
I know this is the way it is currently, but we have an upper level azsdk test and then azsdk pkg test what differs, should we combine
There was a problem hiding this comment.
Oh yeah it should probably just be under pkg entirely.
| var nameSet = new HashSet<string>(StringComparer.OrdinalIgnoreCase); | ||
| foreach (var sub in command.Subcommands) | ||
| { | ||
| if (!nameSet.Add(sub.Name)) |
There was a problem hiding this comment.
Will it end up with same command object if two tools reuse the same verb at different points in their hierarchy?
I'm not entirely sure it makes sense to merge these - I've been thinking of
The other 2 commands don't really seem to have anything to do with TypeSpec IMO, it's more release plan management? |
| ``` | ||
| Tools/ | ||
| ├── Package/ # Package-level operations | ||
| ├── Package/ # Package-level operations |
There was a problem hiding this comment.
Few thoughts on the package command organization:
-
Streamline Code Generation Commands:
generate– Appears underazsdk pkg generatefor SDK code generation and in the TypeSpec workflow (azsdk spec-workflow generate-sdk) for generating an SDK from a TypeSpec project. I thinkgeneratemight fit better under the TypeSpec domain, since it's about turning specs into SDK code, rather than under general package operations.
-
Organization of readme and sample commands: Having a top-level azsdk pkg readme command and a README check as part of
pkg validateis overlapping. Similarly, sample code handling is embedded in validation (“Snippets”) but not as a top level command like Readme . Should README and sample code checks be grouped under a broader release or validation workflow instead of standalone commands? If release-readiness is meant to assess a package’s overall preparedness, it might incorporate ensuring the README and samples are up to date. Alternatively, README and sample updates could be sub-tasks of a release preparation command rather than each being top-level commands. -
Overlapping Release Commands
Thoughts on combiningrelease-readinessinto thereleasecommand group. For example, use a subcommand likerelease checkor an option (e.g.azsdk pkg release --dry-run) to perform readiness checks, instead of a separate top-level command. -
Naming ambiguity (minor concern):
validate– Exists in both groups but with different scope: azsdk pkg validate covers package-level checks (including docs and formatting) while azsdk tsp validate handles TypeSpec spec validation. The name overlap might confuse users even though the validations differ.
There was a problem hiding this comment.
- Does this mean we actually have two duplicate implementations of SDK generation right now, or do these tools differ even though they have the same name? (SpecWorkflowTool and SdkGenerationTool) @raych1 @praveenkuttappan
- Maybe
azsdk pkg release [validate]andazsdk pkg generate [readme/sample]? Something likeazsdk pkg release readmewould indicate we're releasing a readme, not generating one. - I'm for getting rid of
release-readinessjust based on naming alone. validatewithin a sub-command context indicates to me that it's validation for whatever that scope is, not that the validations would be the same. The help text for each should enumerate the possible checks (e.g.All|Changelog|...vs.All|Specs|...)
There was a problem hiding this comment.
I'm for getting rid of release-readiness just based on naming alone.
Ohhh this is interesting. The reason we have this, is so locally people can understand what they are missing or why the release pipeline will fail for them.
tooling uses this too, like for example the Release planner.
Having said that, I like the suggestion @samvaity is giving about an option: azsdk pgk release --dry-run
@praveenkuttappan what do you think?
There was a problem hiding this comment.
Maybe azsdk pkg release [validate] and azsdk pkg generate [readme/sample]? Something like azsdk pkg release readme would indicate we're releasing a readme, not generating one.
@benbp My suggestion would be, to drop the separate pkg readme command. Instead:
azsdk pkg generate – One command to generate necessary artifacts for a package.
azsdk pkg generate(default) – Generate SDK code (from specs, if no specific target given).azsdk pkg generate readme– Generate or update the package’s README file.azsdk pkg generate sample– Generate or update SDK sample code snippets or sample projects.
azsdk pkg validate – Validate various aspects of the package (no change in invocation, but continues to include checks for README format and code snippets among others).
There was a problem hiding this comment.
@benbp yes, we have two generate tools. One is for local experience, and the other is for release scenario using pipeline to generate the SDK. It makes sense to put generate-sdk under spec-workflow since it's part of that flow. I think generate fits better at the package level, as it supports generation from either typespec or an existing SDK project.
There was a problem hiding this comment.
as it supports generation from either typespec or an existing SDK project.
What it means to generate from an existing SDK project?
If I am re-generating is because my spec changed. If I am adding to my sdk it is because I am applying customizations. so it is not generate.
What am I missing?
There was a problem hiding this comment.
@samvaity
Overlaps occur across these assets: changelog, readme, samples, and snippets. We have individual tools for each of them, and the validation tool also provides sub-commands to verify them:
azsdk pkg validate --check-type changelogazsdk pkg validate --check-type readmeazsdk pkg validate --check-type snippets --fix# updates snippets- ...
azsdk pkg validate# validates all checks
For pkg update tools:
azsdk pkg update --target changelog-contentazsdk pkg update --target versionazsdk pkg update --target metadataazsdk pkg update --target ciazsdk pkg update# updates all targets
I think it’s reasonable to have some overlap among these tools. Since the primary users of the CLI are likely automation pipelines, it would actually be more beneficial to provide aggregated commands for convenience. Updates to readme and snippets still wrapped in validation checks, so running pkg update and pkg validate should get everything in a package ready for release.
CC: @maririos
There was a problem hiding this comment.
What it means to generate from an existing SDK project?
It refers to regenerate the SDK in a local SDK repo folder after spec is changed. User can prompt like re-generate code for current project or generate code from {path_or_url_to_the_spec}.
There was a problem hiding this comment.
azsdk pgk release --dry-run
Filed an issue to track this change. #12865
| @@ -90,9 +90,9 @@ For release planning and SDK coordination: | |||
|
|
|||
| ### 3. **typespec** - TypeSpec Operations and TypeSpec Client Operations | |||
There was a problem hiding this comment.
I think there are some gaps breaking the overall flow between tsp and spec-workflow commands. From tehe divided command structure it seems that the users working with TypeSpec must use tsp commands for project setup and validation, then switch to spec-workflow commands for generating the SDK and handling the output.
Should we consider merging the spec-workflow functionality into the tsp group. This would create a single azsdk tsp command group encompassing the entire TypeSpec workflow, from project initiation to SDK generation. For example, azsdk tsp generate could replace spec-workflow generate-sdk, and give a continuous task flow?
There was a problem hiding this comment.
spec-workflow command to link sdk pr will be moved to release plan and another command to generate SDK using pipeline will be deprecated and we can remove spec-workflow hierarchy all together.
There was a problem hiding this comment.
I really like azsdk tsp generate because just with the name it is clear that this won't work for OpenAPI specs. it also shift left the concept of what Typespec brings to the table. and instead of spec-worflow then SDKs? client, thing`
There was a problem hiding this comment.
I don't think we need azsdk tsp generate. This should ideally be merged to azsdk package generate and we can completely deprecate the spec workflow based sdk generation. I will also move link sdk to release plan to release plan group so we can completely deprecate spec-workflow. I will file a separate issue and do it in a separate PR to move link sdk to release plan.
|
@benbp We can remove spec workflow tool class once local SDK generation is in complete state. It has two tools.
|
praveenkuttappan
left a comment
praveenkuttappan
left a comment
There was a problem hiding this comment.
Looks fine to me. We can do any further changes in incremental PR.
yayy 🥳 thanks, I think removing spec-workflow and merging it with tsp group provides advantage of single azsdk tsp command group encompassing the entire TypeSpec workflow |
@samvaity Just to make sure we are all on the same page, my suggestion was to move link sdk pr to release plan. SDK generation currently has currently uses pipeline and is also associated with release plan as an optional property. So, I don't think this completely fit in tsp group. We can keep as it is and retire this tool completely when current local sdk generation is fully operational. |
Filed issue to track this change. #12864 |
|
Changes merged via #12854 |
github-project-automation
Bot
moved this from 🔬 Dev in PR
to 🎊 Closed
in Azure SDK EngSys 🚀🌒🧑🚀
ronniegeraghty
added
the
AzSDK Tools Agent
This PR takes a first pass at organizing our command line hierarchy. This ensures alignment with #12071 as well as organizing the rest of the CLI commands not specified in that doc.
One area that I'm still not sure of is that we can probably merge the top-level
spec-workflowandtypespeccommands, since they are both typespec related? @praveenkuttappan @maririos @chrisradek thoughts?The new structure (levels 1 and 2):