Conversation
Refined Azure Blob Storage API calls by eliminating redundant `cancellationToken` assignments. This ensures cleaner and more concise code.
Added the `LibGit2Sharp` package (v0.31.0) to the `_atom` project for enhanced Git repository interaction capabilities.
…n labels Updated workflows to use `WorkflowLabels` for platform, framework, and version definitions for better consistency and readability. Removed unused classes and redefined multiple injection options. Migrated `GithubIfBuild` test cases and adjusted workflow test cases accordingly.
Added `DecSm.Atom.Workflows.Definition` to global usings for improved accessibility across the module.
Refactored `WithGithubRunsOnMatrix` and `WithGithubTokenInjection` methods for improved consistency and readability. Updated `GithubRunsOn` to use `WorkflowExpression` for labels. Removed redundant static properties for predefined runner configurations.
Added `EnvVarName` computed property to `ParamModel` and `ParamDefinition` for consistent environment variable naming. Updated `ParamService` to leverage the new property, improving readability and reducing duplication.
Enhanced workflow generation logic by introducing `IWorkflowExpressionGenerator` for improved expression handling and consistency. Refactored multiple areas, including runs-on labels, environment variables, and secret injections, to leverage the new generator. Simplified property access and reduced redundancy across GitHub and DevOps workflow writers.
…ndancy Removed redundant static properties and refactored workflow option records for improved clarity and consistency. Simplified the structure of options like `ToggleWorkflowOption`, `GithubCheckoutOption`, and `WorkflowSecretsInjection` by standardizing their definitions. Updated namespaces to align with the new organization.
…tions Removed the `IGithubExpression` record and its related implementations as they were no longer necessary. This includes literals, operators, function expressions, and tests under `ExpressionsTests`. Streamlined the codebase to improve maintainability and reduce unused types.
Simplified the `WithDevopsPoolMatrix` method implementation for improved clarity and consistency. Updated parameter handling and adjusted namespace dependencies to align with recent workflow architecture changes.
…ng change checks Introduced new workflow expression functionalities like `Not`, `And`, `Or`, `Coalesce`, and others to enhance GitHub workflow definition capabilities. Added logic for checking pull requests against potential breaking changes using the public API surface. Integrated `ICheckPrForBreakingChanges` and `IWorkflowExpressionWriter` for improved workflow management and customization.
Moved `CheckPrForBreakingChanges` logic in `Build.cs` to a specific workflow section and updated its injection configuration. Ensured consistency with GitHub token permissions and matrix setup. Removed redundant duplicate declarations for improved clarity.
There was a problem hiding this comment.
Pull request overview
This PR introduces a major refactoring that modernizes the workflow expression system and API design patterns. The main changes include:
Changes:
- Introduces a new type-safe
WorkflowExpressionhierarchy to replace the old GitHub-specific expression system - Migrates from generic
WorkflowOption<TData, TSelf>base classes to more specific record types with primary constructors - Adds a new
CheckPrForBreakingChangestarget that uses LibGit2Sharp to detect breaking API changes - Introduces extension-based API patterns (
WorkflowOptions,WorkflowTriggers,WorkflowLabels) for better discoverability - Renames generated property from
ParamstoWorkflowParamsto avoid naming conflicts - Updates parameter environment variable name generation to include additional character replacements
Reviewed changes
Copilot reviewed 104 out of 104 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| _atom/_usings.cs | Adds new global usings for expressions and dotnet module |
| _atom/_atom.csproj | Adds LibGit2Sharp package reference |
| _atom/Targets/IPullRequestHelper.cs | New interface for PR number parameter |
| _atom/Targets/ICheckPrForBreakingChanges.cs | New target to detect breaking API changes via git diff |
| _atom/Build.cs | Major refactoring to use new WorkflowOptions, WorkflowTriggers, and WorkflowLabels APIs |
| DecSm.Atom/_usings.cs | Adds usings for new expressions and injection namespaces |
| DecSm.Atom/Workflows/Options/* | New extension-based API for workflow options |
| DecSm.Atom/Workflows/Expressions/* | New expression system with generator pattern |
| DecSm.Atom/Workflows/Definition/Options/WorkflowOption.cs | Deleted generic base class |
| DecSm.Atom/Workflows/Definition/Options/ToggleWorkflowOption.cs | Simplified to remove static instances |
| DecSm.Atom/Params/* | Adds EnvVarName computed property |
| DecSm.Atom/Hosting/HostExtensions.cs | Registers new expression generator service |
| DecSm.Atom.SourceGenerators/* | Renames Params to WorkflowParams |
| DecSm.Atom.Module.GithubWorkflows/* | Implements GitHub expression writer and new extension APIs |
| DecSm.Atom.Module.DevopsWorkflows/* | Implements DevOps-specific extensions |
| DecSm.Atom.Module.*/WorkflowOptionsExtensions.cs | New extension files for each module |
| .github/workflows/Build.yml | Adds CheckPrForBreakingChanges job |
Comments suppressed due to low confidence (1)
.github/workflows/Build.yml:217
- The CheckPrForBreakingChanges target is added to the Build workflow, but the job is missing a conditional check. According to the PushToRelease target (line 322), releases only occur when the build version does not contain a '-' character. The CheckPrForBreakingChanges job should likely have a similar condition to only run on pull requests, not on releases or regular builds.
PushToNuget:
needs: [ TestProjects, PackProjects, PackTool, SetupBuildInfo ]
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-dotnet@v4
with:
dotnet-version: '10.0.x'
- name: Download DecSm.Atom
uses: actions/download-artifact@v4
with:
name: DecSm.Atom
path: "${{ github.workspace }}/.github/artifacts/DecSm.Atom"
- name: Download DecSm.Atom.Module.AzureKeyVault
uses: actions/download-artifact@v4
with:
name: DecSm.Atom.Module.AzureKeyVault
path: "${{ github.workspace }}/.github/artifacts/DecSm.Atom.Module.AzureKeyVault"
- name: Download DecSm.Atom.Module.AzureStorage
uses: actions/download-artifact@v4
with:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Updated `GithubWorkflowWriter` to include `workflowStep.Options` in the processing of environment and parameter injections. Ensured that environment injections correctly exclude matching parameter injections to maintain expected behavior.
Updated `Validate.yml` and `Build.cs` to include `pull-request-number` parameter for PR breaking change checks. Ensured proper injection of GitHub event data to support enhanced workflow logic.
|
✅ No Breaking Changes Detected This pull request does not contain any breaking changes to the public API surface. |
1 similar comment
|
✅ No Breaking Changes Detected This pull request does not contain any breaking changes to the public API surface. |
Refactored logic in `ICheckPrForBreakingChanges` to enhance clarity and functionality. Introduced separate handling for breaking change comments and non-breaking change comments. Prevented redundant mutation executions and improved error handling for mutation results.
|
✅ No Breaking Changes Detected This pull request does not contain any breaking changes to the public API surface. |
1 similar comment
|
✅ No Breaking Changes Detected This pull request does not contain any breaking changes to the public API surface. |
Added `AtomFileSystem.cs` to the list of files checked for breaking changes in `ICheckPrForBreakingChanges`. Improved repository variable handling by splitting and formatting GitHub repository strings.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request contains breaking changes to the public API surface.
Please review the following changes carefully:
/home/runner/work/atom/atom/DecSm.Atom/Paths/AtomFileSystem.cs: 2 line(s) removed or modified
Action Required:
- Ensure the version bump is appropriate (major version for breaking changes)
- Update migration documentation if necessary
- Verify all consumers can handle these changes
There was a problem hiding this comment.
This pull request contains breaking changes to the public API surface.
Please review the following changes carefully:
/home/runner/work/atom/atom/DecSm.Atom/Paths/AtomFileSystem.cs: 2 line(s) removed or modified
Action Required:
- Ensure the version bump is appropriate (major version for breaking changes)
- Update migration documentation if necessary
- Verify all consumers can handle these changes
Improved handling of breaking change detection by introducing clearer formatting of repository variables and generating detailed draft PR review threads. Included file-specific line details for breaking changes to assist reviewers in identifying affected areas. Removed redundant code execution for review thread creation.
…flows Centralized breaking change detection logic into `SetupBuildInfo` and removed `ICheckPrForBreakingChanges` target to streamline workflow execution. Enhanced PR review comment generation with detailed breaking change insights. Updated workflows to ensure proper permissions and environment variable handling for seamless integration.
Enhanced workflows by injecting Azure Vault secrets and environment variables to support secure operations. Refactored breaking change detection in PR reviews for better clarity and streamlined mutation execution.
…ndling Enhanced GitHub repository variable handling for improved readability by splitting and formatting `.Variables.Repository` more clearly. Adjusted breaking change detection logic to use relative paths with consistent forward slashes for generating PR review threads.
…logic Refactored repository variable formatting for better readability by splitting and aligning `.Variables.Repository`. Enhanced logic for breaking change detection by adding relative path computation and consistent forward slashes. Introduced logs for path-specific breaking changes and improved draft PR review thread generation to include detailed file and line information.
Improved readability of repository variable formatting by aligning GitHub `.Variables.Repository`. Enhanced breaking change logic by trimming paths, ensuring consistent forward slashes, and wrapping path references in single quotes for better accuracy.
Introduced structured breaking change types (major, minor) with corresponding comment generation. Added methods for formatting target files and identifying the latest release version. Refactored breaking change logic to provide actionable insights, improving review clarity and version bump detection.
…handling Added debug logs to improve traceability of change detection steps, including repository owner, version, commit hashes, and change classification (major/minor). Consolidated breaking change handling into a new `Change` record for clearer structure. Refined detection of suspicious and major changes with improved logging of intermediate results, enhancing review clarity.
Simplified code by removing unused logic for checking suspicious changes that prevented comment mutation execution. This ensures the comment mutation always occurs, improving clarity and consistency.
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
1 similar comment
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
…lper Extracted breaking change detection and related utilities from `ISetupBuild` into a new `IApiSurfaceHelper` interface for better modularity and code reuse. Updated workflows and targets to leverage the new helper. Removed redundant environment variables from GitHub workflows for improved clarity.
Introduced a method to create GitHub check runs for detecting breaking changes in pull requests. The check dynamically determines success or failure based on categorized (major/minor) breaking changes and current version information. Enhanced PR review clarity with additional status updates and descriptions.
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
1 similar comment
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
Added the `checks: write` permission to the `Validate.yml` workflow and corresponding logic in `Build.cs` to support creating and updating GitHub check runs. This improves integration with GitHub Actions for enhanced validation and status reporting.
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
1 similar comment
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
Simplified logic for validating breaking changes by introducing a concise predicate to determine invalid changes based on major/minor version comparisons. Replaced redundant switch expression with direct variable usage for check run generation. Improved pull request query to leverage accurate head ref SHA for GitHub check runs. Enhanced readability of repository variable formatting and ensured proper timestamps in check run mutations.
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
1 similar comment
|
This pull request contains major breaking changes to the public API surface, but the major version has not been bumped. Current Version: Files with breaking changes:
Required Action: Please increment the major version number before merging this pull request. |
Eliminated unnecessary PR comment handling logic and redundant status checks for major/minor version bumps. Simplified `CheckPrForBreakingChanges` by refining body generation and ensuring clear differentiation between success and failure statuses. Improved readability of repository variable formatting.
Enhanced breaking change detection messages to provide clear action requirements for major/minor version bumps when discrepancies are identified. Introduced structured outputs for detailed file and line impact reporting. Refined logic for determining invalid changes and updated repository variable formatting for improved readability.
…eakingChanges` Moved `FindLatestReleaseInfo` method from `IApiSurfaceHelper` to `ICheckPrForBreakingChanges` to improve cohesion and align the method's functionality with its intended usage. Removed redundant `ReleaseInfo` record definition from `IApiSurfaceHelper` and consolidated it within `ICheckPrForBreakingChanges`. Simplified repository variable formatting during refactor.
1 participant
This pull request introduces several improvements and refactorings across the Azure Key Vault and DevOps Workflows modules, as well as updates to workflow definitions and CI/CD pipeline configuration. The main focus is on standardizing workflow option usage, enhancing extensibility, and improving maintainability. Key highlights include a new workflow option extension for Azure Key Vault, migration to a unified
WorkflowOptionsAPI in test and production code, and updates to the DevOps workflow writer for better expression handling.Azure Key Vault Integration and Refactoring:
WorkflowOptionsExtensionsclass inDecSm.Atom.Module.AzureKeyVaultto provide a standardized way to access Azure Key Vault workflow options, improving discoverability and consistency.AzureKeySecretsProvider.csto use the newWorkflowOptions.Inject.SecretFromWorkflowEnvironmentandSecretForSecretProvidermethods, replacing the previousWorkflowSecretsEnvironmentInjectionandWorkflowSecretsSecretInjectionusage for secret injections. [1] [2] [3] [4]DevOps Workflows Standardization and Enhancements:
DevopsCheckoutOptionclass and migrated all test workflow definitions to use the newWorkflowOptionsAPI, ensuring a consistent approach for configuring workflow options like custom artifact providers, checkout options, and deployment environments. [1] [2] [3] [4] [5] [6] [7]Extensions.csto use the newWorkflowOptions.Devops.DevopsPool.SetByMatrix, streamlining how DevOps pool matrices are configured for workflow targets.DevOps Workflow Writer Improvements:
DevopsWorkflowWriterto use aworkflowExpressionGeneratorfor rendering pool options (HostedImage,Name,Demands) and environment names, allowing for more flexible and dynamic workflow YAML generation. [1] [2] [3]CI/CD Pipeline Updates:
CheckPrForBreakingChangesjob in the GitHub Actions workflow to automate breaking change checks on pull requests, improving release safety.PushToReleasejob in the GitHub Actions workflow for better readability.Minor Fixes and Cleanups:
These changes collectively modernize the workflow configuration experience, reduce duplication, and make the codebase easier to extend and maintain.