Skip to content

Conversation

isra-fel
Copy link
Member

Description

This pull request introduces a new feature to the AzDev toolset that enables detection and reporting of the AutoRest.PowerShell version (v3 or v4) used in AutoRest-based projects. This version is now surfaced as the SubType property in both the project model and the Get-DevProject output, allowing users to easily group or filter projects by AutoRest version. The PR also adds comprehensive tests for this functionality and updates documentation and formatting to support the new property.

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

@Copilot Copilot AI review requested due to automatic review settings August 25, 2025 15:42
Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copilot

This comment was marked as outdated.

@isra-fel isra-fel requested a review from Copilot August 26, 2025 05:42
Copy link
Contributor

@Copilot 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 introduces a new feature to the AzDev toolset that automatically detects and reports the AutoRest.PowerShell version (v3 or v4) used in AutoRest-based projects. The detected version is exposed as a new SubType property in both the project model and Get-DevProject cmdlet output, enabling users to easily group and filter projects by AutoRest version.

Key changes include:

  • AutoRest.PowerShell version detection logic that parses YAML configurations in README.md files
  • New SubType property added to project models to store the detected version
  • Enhanced YAML parsing capabilities with error handling and version mapping logic

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/AzDev/src/Services/YamlHelper.cs Adds safe YAML deserialization method with exception handling
tools/AzDev/src/Services/Conventions.cs Implements version mapping logic for AutoRest.PowerShell versions
tools/AzDev/src/Models/PSModels/PSProject.cs Adds SubType property to PowerShell project model
tools/AzDev/src/Models/Inventory/Project.cs Adds SubType property to base project model
tools/AzDev/src/Models/Inventory/AutoRestYamlConfig.cs Enhances YAML config model to parse use-extension blocks
tools/AzDev/src/Models/Inventory/AutoRestProject.cs Implements AutoRest version detection and refactors configuration loading
tools/AzDev/Tests/PSTests/InventoryTests.ps1 Adds test to validate SubType property on AutoRest projects
tools/AzDev/Tests/ModelTests/ProjectTests.cs Adds comprehensive unit tests for version detection logic
tools/AzDev/Tests/ModelTests/ModuleTests.cs Updates existing tests to include README.md files
tools/AzDev/Tests/HelperTests/YamlTests.cs Updates YAML tests for directive property changes
tools/AzDev/Tests/HelperTests/ConventionTests.cs Adds tests for version mapping function
tools/AzDev/README.md Updates documentation with examples of new SubType functionality
tools/AzDev/CHANGELOG.md Documents the new feature
tools/AzDev/AzDev/AzDev.format.ps1xml Adds SubType column to default display format
Comments suppressed due to low confidence (1)

tools/AzDev/src/Models/Inventory/AutoRestYamlConfig.cs:1

  • Line 44 shows a removed property of type IEnumerable<object> but line 45 adds a property of the same name with type object. This type change will break existing code that expects Directive to be enumerable. Consider keeping the original type or using a different property name.
// ----------------------------------------------------------------------------------

Comment on lines +123 to +124
.Select(y => YamlHelper.TryDeserialize<AutoRestYamlConfig>(y, out var c) ? c : null)
.Where(c => c != null)!;
Copy link
Preview

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

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

The null-forgiving operator ! at the end of line 124 is unnecessary and potentially misleading since the Where(c => c != null) filter guarantees no null values remain in the sequence. Remove the ! operator.

Suggested change
.Select(y => YamlHelper.TryDeserialize<AutoRestYamlConfig>(y, out var c) ? c : null)
.Where(c => c != null)!;
.Where(c => c != null);

Copilot uses AI. Check for mistakes.

{
yamlBlocks.Add(match.Groups[1].Value.Trim());
var kvp = c.UseExtension.FirstOrDefault(k => k.Key.Replace("\"", string.Empty).Equals("@autorest/powershell", StringComparison.OrdinalIgnoreCase));
Copy link
Preview

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

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

The string replacement logic k.Key.Replace(\"\\\", string.Empty)` appears to be handling quoted keys in YAML. This should be handled by the YAML deserializer instead of manual string manipulation. Consider configuring the YAML deserializer to handle quotes properly.

Suggested change
var kvp = c.UseExtension.FirstOrDefault(k => k.Key.Replace("\"", string.Empty).Equals("@autorest/powershell", StringComparison.OrdinalIgnoreCase));
var kvp = c.UseExtension.FirstOrDefault(k => k.Key.Equals("@autorest/powershell", StringComparison.OrdinalIgnoreCase));

Copilot uses AI. Check for mistakes.

@VeryEarly VeryEarly self-assigned this Aug 28, 2025
@VeryEarly VeryEarly merged commit 8b3521a into Azure:main Aug 28, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants