Skip to content

refactor --check-type and --package-path and service initialization for language repos#11779

Merged
l0lawrence merged 31 commits into
Azure:mainfrom
l0lawrence:refactorservice
Aug 27, 2025
Merged

refactor --check-type and --package-path and service initialization for language repos#11779
l0lawrence merged 31 commits into
Azure:mainfrom
l0lawrence:refactorservice

Conversation

@l0lawrence
Copy link
Copy Markdown
Member

No description provided.

@github-actions github-actions Bot added the azsdk-cli Issues related to Azure/azure-sdk-tools::tools/azsdk-cli label Aug 21, 2025
Comment thread tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/Languages/ILanguageSpecificChecks.cs Outdated
Comment thread tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/Languages/LanguageChecks.cs Outdated
Comment thread tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/ChecksHelper.cs Outdated
Comment thread tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/Languages/LanguageChecks.cs Outdated
Comment thread tools/azsdk-cli/Azure.Sdk.Tools.Cli/Commands/SharedOptions.cs Outdated
Comment thread tools/azsdk-cli/Azure.Sdk.Tools.Cli/Commands/SharedOptions.cs
@l0lawrence l0lawrence changed the title refactor refactor --check-type and --package-path and service initialization for language repos Aug 27, 2025
@l0lawrence l0lawrence marked this pull request as ready for review August 27, 2025 16:35
Copilot AI review requested due to automatic review settings August 27, 2025 16:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the package checking tool to replace the factory pattern with a composition-based approach for language-specific validations. The main goal is to modernize the architecture by using dependency injection and a more flexible language detection mechanism.

  • Replaced --check-type option parameter with individual sub-commands for each check type
  • Changed --package-path option to default to current directory instead of being required
  • Replaced inheritance-based language services with composition-based language-specific checks

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
PackageCheckTool.cs Refactored command structure to use sub-commands instead of options, updated to use ILanguageChecks interface
ServiceRegistrations.cs Updated DI container to register new composition-based language check services
LanguageSpecificCheckResolver.cs New resolver service for detecting and selecting appropriate language-specific check implementations
PythonLanguageSpecificChecks.cs New composition-based Python language check implementation
JavaScriptLanguageSpecificChecks.cs New composition-based JavaScript language check implementation
JavaLanguageSpecificChecks.cs New composition-based Java language check implementation
GoLanguageSpecificChecks.cs New composition-based Go language check implementation
DotNetLanguageSpecificChecks.cs New composition-based .NET language check implementation
LanguageChecks.cs New unified language checks service that delegates to language-specific implementations
ILanguageSpecificChecks.cs New interface defining the contract for language-specific check implementations
CLICheckResponse.cs Added utility method for creating responses from process results
SharedOptions.cs Updated PackagePath option to use current directory as default and made it optional
Comments suppressed due to low confidence (1)

tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/Package/PackageCheckTool.cs:31

  • The SupportedLanguage property returns "JavaScript" but the LanguageSpecificCheckResolver expects language names to match what's extracted from Language-Settings.ps1. Based on the resolver's detection logic, this should likely be "js" or another value that matches the PowerShell script output.
            this.languageChecks = languageChecks;

l0lawrence and others added 4 commits August 27, 2025 09:47
Co-authored-by: Timo van Veenendaal <timov@microsoft.com>
…iptLanguageSpecificChecks.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread tools/azsdk-cli/Azure.Sdk.Tools.Cli/Models/Responses/CLICheckResponse.cs Outdated
Copy link
Copy Markdown
Member

@benbp benbp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits, LGTM

@l0lawrence l0lawrence merged commit d2dfe8f into Azure:main Aug 27, 2025
13 checks passed
l0lawrence added a commit to l0lawrence/azure-sdk-tools that referenced this pull request Aug 28, 2025
danieljurek pushed a commit that referenced this pull request Aug 29, 2025
…or language repos (#11779)

* this?

* this

* need to fix

* do not need a default it will do nothing

* remove default

* updating

* update package tool

* add Go

* rename

* add .net and java

* add other lang outline

* spelling

* make a helper func

* nit

* renames

* update checks

* also update CLI tool command to remove check-type

* can_handle higher level, add interfaces for mocking

* remove checkHelper

* make var private

* set default current working directory

* run pwsh command

* Update tools/azsdk-cli/Azure.Sdk.Tools.Cli/Commands/SharedOptions.cs

Co-authored-by: Timo van Veenendaal <timov@microsoft.com>

* dotnet

* Update tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/Languages/JavaScriptLanguageSpecificChecks.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* constructor overload

* Update tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/Languages/GoLanguageSpecificChecks.cs

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

* correcting code

---------

Co-authored-by: Timo van Veenendaal <timov@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <ben@benbp.net>
radhgupta pushed a commit to radhgupta/azure-sdk-tools that referenced this pull request Sep 9, 2025
…or language repos (Azure#11779)

* this?

* this

* need to fix

* do not need a default it will do nothing

* remove default

* updating

* update package tool

* add Go

* rename

* add .net and java

* add other lang outline

* spelling

* make a helper func

* nit

* renames

* update checks

* also update CLI tool command to remove check-type

* can_handle higher level, add interfaces for mocking

* remove checkHelper

* make var private

* set default current working directory

* run pwsh command

* Update tools/azsdk-cli/Azure.Sdk.Tools.Cli/Commands/SharedOptions.cs

Co-authored-by: Timo van Veenendaal <timov@microsoft.com>

* dotnet

* Update tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/Languages/JavaScriptLanguageSpecificChecks.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* constructor overload

* Update tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/Languages/GoLanguageSpecificChecks.cs

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

* correcting code

---------

Co-authored-by: Timo van Veenendaal <timov@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <ben@benbp.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

azsdk-cli Issues related to Azure/azure-sdk-tools::tools/azsdk-cli

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants