-
Notifications
You must be signed in to change notification settings - Fork 227
Refactor Copilot setup workflow for improved clarity and functionality #2140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2140 +/- ##
====================================
Coverage 94% 94%
====================================
Files 110 110
Lines 8171 8171
====================================
Hits 7732 7732
Misses 439 439
🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/copilot-setup-steps.yml(4 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.md
**/*.md
⚙️ CodeRabbit Configuration File
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Surround fenced code blocks with blank lines
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters
Files:
CHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit Configuration File
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe main changes as concise release notes
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
🪛 YAMLlint (1.37.1)
.github/workflows/copilot-setup-steps.yml
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Publish Code Coverage)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (2)
.github/workflows/copilot-setup-steps.yml (2)
32-65: Robust shallow-check for GitVersion is solidThe conditional unshallow/tag fetch with exit-code checks is a good hardening against Copilot’s shallow clones. No issues spotted.
118-126: Idempotent .NET tool setup and GitVersion verification look goodUsing
dotnet tool update || installplus explicit verification is correct and repeatable.Also applies to: 129-136
…undant environment variable settings in DSCv3 steps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
.github/workflows/copilot-setup-steps.yml (3)
2-3: Allow overriding MODULE_NAME via workflow_dispatch inputLet consumers override the module name on manual runs without editing the file. Keep ‘SqlServerDsc’ as default for PR/push.
env: - MODULE_NAME: 'SqlServerDsc' + MODULE_NAME: ${{ inputs.module_name || 'SqlServerDsc' }} on: - workflow_dispatch: + workflow_dispatch: + inputs: + module_name: + description: 'PowerShell module name to build/import' + required: false + default: 'SqlServerDsc'Also applies to: 8-16
182-184: Import built module by manifest path to avoid pulling an installed versionImporting by name can resolve to an already installed module on PSModulePath. Import the freshly built manifest explicitly.
- ./build.ps1 -Tasks 'noop' - Import-Module -Name $env:MODULE_NAME -Force + ./build.ps1 -Tasks 'noop' + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) + $manifest = Join-Path -Path $builtPath -ChildPath "$env:MODULE_NAME.psd1" + if (-not (Test-Path -Path $manifest)) { + Write-Error "Module manifest not found at: $manifest" + exit 1 + } + Import-Module -FullyQualifiedName $manifest -ForceAlso applies to: 185-189
210-213: Don’t fail the job if DSC resource listing isn’t availableIf the adapter/CLI isn’t present, make this informational step non-fatal.
- $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) - Write-Host " Available class-based DSC Resources: $($dscResources.Count)" -ForegroundColor Cyan - $dscResources + try { + $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) + Write-Host " Available class-based DSC Resources: $($dscResources.Count)" -ForegroundColor Cyan + $dscResources + } catch { + Write-Warning "Failed to list DSC resources: $($_.Exception.Message)" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/copilot-setup-steps.yml(3 hunks).vscode/settings.json(1 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Always update the Unreleased section in CHANGELOG.md using Keep a Changelog format with concrete release notes for changes (new/modified commands, functions, resources)
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe main changes as concise release notes
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Markdown: wrap lines at word boundaries when exceeding 80 characters; use 2 spaces for indentation
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Surround fenced code blocks with blank lines
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters
Files:
CHANGELOG.md
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.md
🪛 YAMLlint (1.37.1)
.github/workflows/copilot-setup-steps.yml
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (1)
.vscode/settings.json (1)
92-95: Additions to cSpell dictionary look goodThe new terms “libmi” and “libc” are relevant for DSC/OMI contexts and prevent false positives during spell checks.
…roved error handling
…ion and improved dependency handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
CHANGELOG.md (1)
18-21: Duplicate Copilot workflow notes under Added vs Changed — remove the Added entryThe Copilot setup workflow wasn’t added in this release; it’s being refactored. Keeping both entries duplicates the same change in Unreleased, violating the “no duplication” guideline.
Apply this diff to remove the duplicate from Added:
- - Added setup workflow for GitHub Copilot. - - Switch the workflow to use Linux. - - Attempt to unshallow the Copilot branch - - Improved AI instructions..github/workflows/copilot-setup-steps.yml (4)
2-3: Parameterize MODULE_NAME via workflow_dispatch input (keeps default for PR/push)Allow overriding the module name on manual runs without editing the file. Default remains SqlServerDsc for PR/push.
env: - MODULE_NAME: 'SqlServerDsc' + MODULE_NAME: ${{ inputs.module_name || 'SqlServerDsc' }} on: workflow_dispatch: + inputs: + module_name: + description: 'PowerShell module name to build/import' + required: false + default: 'SqlServerDsc' pull_request: paths: - '.github/workflows/copilot-setup-steps.yml' push: paths: - '.github/workflows/copilot-setup-steps.yml'Also applies to: 8-16
189-205: Import the built module by manifest path; avoid picking an installed versionImporting by name can load a preinstalled module from PSModulePath instead of the freshly built bits. Import the manifest from the build output.
./build.ps1 -Tasks 'noop' - Import-Module -Name $env:MODULE_NAME -Force + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) + $manifest = Join-Path -Path $builtPath -ChildPath "$env:MODULE_NAME.psd1" + if (-not (Test-Path -Path $manifest)) { + Write-Error "Module manifest not found at: $manifest" + exit 1 + } + Import-Module -FullyQualifiedName $manifest -Force @@ - # Verify module is loaded - $module = Get-Module -Name $env:MODULE_NAME -ErrorAction SilentlyContinue + # Verify module is loaded + $module = Get-Module -Name $env:MODULE_NAME -ErrorAction SilentlyContinue @@ - # Show available commands - $commands = @(Get-Command -Module $env:MODULE_NAME) + # Show available commands + $commands = @(Get-Command -Module $env:MODULE_NAME) if ($commands.Count -gt 0)
219-221: Make DSC resource listing non-fatalIf the adapter/CLI changes or isn’t present, this informational call can fail a successful build/import. Wrap it.
- $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) - Write-Host " Available class-based DSC Resources: $($dscResources.Count)" -ForegroundColor Cyan - $dscResources + try { + $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) + Write-Host " Available class-based DSC Resources: $($dscResources.Count)" -ForegroundColor Cyan + $dscResources + } + catch { + Write-Warning "Failed to list DSC resources: $($_.Exception.Message)" + }
169-183: Optional: Use Join-Path for build output paths for portability/readabilityPowerShell-native path joins reduce ambiguity and align with “Linux-friendly paths,” especially in messages and subsequent Get-ChildItem.
- # Verify build output - if (Test-Path -Path "output/builtModule/$env:MODULE_NAME") + # Verify build output + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) + if (Test-Path -Path $builtPath) { - Write-Host "Module built successfully at: output/builtModule/$env:MODULE_NAME" -ForegroundColor Green - Get-ChildItem -Path "output/builtModule/$env:MODULE_NAME" -Recurse | Select-Object Name, Length | Format-Table + Write-Host "Module built successfully at: $builtPath" -ForegroundColor Green + Get-ChildItem -Path $builtPath -Recurse | Select-Object Name, Length | Format-Table } else { Write-Error 'Module build failed - output directory not found' exit 1 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/copilot-setup-steps.yml(3 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Always update the Unreleased section in CHANGELOG.md using Keep a Changelog format with concrete release notes for changes (new/modified commands, functions, resources)
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe main changes as concise release notes
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Markdown: wrap lines at word boundaries when exceeding 80 characters; use 2 spaces for indentation
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Surround fenced code blocks with blank lines
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters
Files:
CHANGELOG.md
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.md
🧠 Learnings (1)
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to CHANGELOG.md : Always update the Unreleased section in CHANGELOG.md using Keep a Changelog format with concrete release notes for changes (new/modified commands, functions, resources)
Applied to files:
CHANGELOG.md
🪛 YAMLlint (1.37.1)
.github/workflows/copilot-setup-steps.yml
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (3)
CHANGELOG.md (1)
53-55: LGTM: Clear, Keep a Changelog–compliant refactor entryThe “Changed” bullet succinctly captures the module-agnostic refactor, full-history handling, idempotent .NET tool install, and Linux dependency setup with issue reference.
.github/workflows/copilot-setup-steps.yml (2)
32-65: LGTM: Robust shallow check before GitVersionGood guard around shallow repos with explicit tag fetch and error handling. This mitigates the Copilot branch depth issue.
151-154: LGTM: GitVersion verification outputRunning both the raw and JSON-parsed outputs is helpful for diagnostics.
… commands and checking for Microsoft repository before installation
…t repository check, and error handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/copilot-setup-steps.yml (4)
2-3: Make MODULE_NAME overridable via workflow_dispatch input (keeps default for PR/push)Allow manual runs to override the module name without editing the file. Keep the default for automated triggers.
Apply this diff to switch env to use the input when provided:
env: - MODULE_NAME: 'SqlServerDsc' + MODULE_NAME: ${{ inputs.module_name || 'SqlServerDsc' }}And add the input under workflow_dispatch (outside this hunk):
on: workflow_dispatch: inputs: module_name: description: 'PowerShell module name to build/import' required: false default: 'SqlServerDsc'
231-245: Make DSC resource listing non-fatal (optional)If the adapter/CLI isn’t present or output format changes, this informational call can fail the job. Wrap with try/catch and warn.
Apply this diff:
# Show available commands $commands = @(Get-Command -Module $env:MODULE_NAME) if ($commands.Count -gt 0) { Write-Host " Exported Commands: $($commands.Count)" Write-Host 'Available Commands:' $commands | Select-Object Name, ModuleName | Format-Table -AutoSize } else { Write-Host 'No commands exported by the module.' } - $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) - Write-Host " Available class-based DSC Resources: $($dscResources.Count)" - $dscResources + try { + $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) + Write-Host " Available class-based DSC Resources: $($dscResources.Count)" + $dscResources + } catch { + Write-Warning "Failed to list DSC resources: $($_.Exception.Message)" + }Also applies to: 246-249
218-223: Import the built module by manifest path to avoid pulling an installed versionImporting by name can load an older installed module from PSModulePath. Import the freshly built manifest explicitly and fail early if it’s missing.
Apply this diff:
- ./build.ps1 -Tasks 'noop' - Import-Module -Name $env:MODULE_NAME -Force + ./build.ps1 -Tasks 'noop' + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) + $manifest = Join-Path -Path $builtPath -ChildPath "$env:MODULE_NAME.psd1" + if (-not (Test-Path -Path $manifest)) { + Write-Error "Module manifest not found at: $manifest" + exit 1 + } + Import-Module -FullyQualifiedName $manifest -Force
66-126: Harden Microsoft repo add and LD_LIBRARY_PATH exportTwo small resiliency tweaks:
- Guard apt-get update after adding the Microsoft repo to avoid job failure on unsupported codenames (e.g., noble).
- Quote $GITHUB_ENV when appending to it.
Apply this diff:
# Update package list again after adding the new repo echo "Updating package list after adding Microsoft repository..." - sudo apt-get update - echo "Microsoft repository added successfully" + if ! sudo apt-get update; then + echo "::warning::apt-get update failed after adding Microsoft repo (repo may be unavailable for this distro). Continuing..." + else + echo "Microsoft repository added successfully" + fi echo "::endgroup::" @@ # Set LD_LIBRARY_PATH permanently for the runner session echo "Setting LD_LIBRARY_PATH environment variable..." - echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> $GITHUB_ENV + echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> "$GITHUB_ENV" echo "Environment variable set successfully"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/copilot-setup-steps.yml(2 hunks).vscode/settings.json(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/copilot-setup-steps.yml
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (11)
.vscode/settings.json (1)
92-98: LGTM: New cSpell terms align with workflow additionsThe added words match the new Linux/Actions terminology introduced in the workflow (OMI/libmi, keyrings, dearmor, endgroup, distro). No issues.
.github/workflows/copilot-setup-steps.yml (10)
32-65: LGTM: Robust full-history check for GitVersionGood use of rev-parse to detect shallow clones and explicit tag refresh. The fallback to --tags for non-shallow repos is correct.
130-142: LGTM: PS module setup messagingThe added Write-Host diagnostics improve visibility. PSDSC is installed prior to Install-DscExe in the next step, which is correct.
146-154: LGTM: DSCv3 installation orderInstall-DscExe after PSDSC is the right sequence. Good explicit messaging.
158-160: LGTM: Quick DSC sanity checkSimple version check is appropriate here.
164-174: LGTM: Idempotent GitVersion.Tool installUsing update with install fallback is clean. Pinning to 5.* keeps compatibility with existing GitVersion usage.
178-181: LGTM: GitVersion verificationPlain output and JSON parse cover both human and script diagnostics.
186-192: LGTM: Dependency resolutionResolveDependency before build is standard for this repo’s build pipeline.
5-5: LGTM: Clarifying workflow commentThe added description clarifies Copilot’s usage context and why additional steps exist. Good doc hygiene.
17-17: LGTM: cSpell directiveIgnoring “unshallow”, “LASTEXITCODE”, and “PSDSC” reduces noise in editor spell checks.
20-20: LGTM: Job name generalizedRenaming to “Setup PowerShell Development Environment” reflects the new module-agnostic design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (4)
.github/workflows/copilot-setup-steps.yml (4)
219-262: Import built module by manifest path to avoid picking installed versionsImporting by name can load a previously installed module. Import the built manifest explicitly. Also, keep your good use of array cast for commands.
Apply this diff:
./build.ps1 -Tasks 'noop' - Import-Module -Name $env:MODULE_NAME -Force + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) + $manifest = Join-Path -Path $builtPath -ChildPath "$env:MODULE_NAME.psd1" + if (-not (Test-Path -Path $manifest)) { + Write-Error "Module manifest not found at: $manifest" + exit 1 + } + Import-Module -FullyQualifiedName $manifest -Force @@ - $commands = @(Get-Command -Module $env:MODULE_NAME) - if ($commands.Count -gt 0) + $commands = @(Get-Command -Module $env:MODULE_NAME) + if ($commands.Count -gt 0) { Write-Host " Exported Commands: $($commands.Count)" Write-Host "`nAvailable Commands:" @@ - $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) - Write-Host "`n Available class-based DSC Resources: $($dscResources.Count)`n" - $dscResources | Format-Table type, capabilities, implementedAs, requireAdapter -GroupBy kind -AutoSize + try { + $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) + Write-Host "`n Available class-based DSC Resources: $($dscResources.Count)`n" + $dscResources | Format-Table type, capabilities, implementedAs, requireAdapter -GroupBy kind -AutoSize + } catch { + Write-Warning "Failed to list DSC resources: $($_.Exception.Message)" + }
2-3: Parameterize MODULE_NAME via workflow_dispatch input (optional)Allow overriding the module name on manual dispatch without editing the file; keep the existing default for PR/push.
Apply this diff:
name: 'Copilot Setup Steps' -env: - MODULE_NAME: 'SqlServerDsc' +env: + MODULE_NAME: ${{ inputs.module_name || 'SqlServerDsc' }} @@ -on: - workflow_dispatch: + 'on': + workflow_dispatch: + inputs: + module_name: + description: 'PowerShell module name to build/import' + required: false + default: 'SqlServerDsc'
120-124: Quote $GITHUB_ENV when exporting LD_LIBRARY_PATHQuote the environment file path to avoid issues with spaces; you’re already appending to preserve existing values, which is good.
Apply this diff:
- echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> $GITHUB_ENV + echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> "$GITHUB_ENV"
201-212: Prefer Join-Path for cross-platform clarity (optional)Use Join-Path instead of string-based paths for readability and platform neutrality.
Apply this diff:
- $builtPath = "output/builtModule/$env:MODULE_NAME" + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) if (Test-Path -Path $builtPath) { Write-Host "Module built successfully at: $builtPath" - # Show files with paths relative to the built module base path using `tree` (run directly in PowerShell) - $base = (Resolve-Path -Path $builtPath).ProviderPath - Write-Host "Files under $base (relative paths):" - - # Run tree directly and strip leading './' from its output - tree --noreport $builtPath + # Show files with paths relative to the built module base path using `tree` + $base = (Resolve-Path -Path $builtPath).ProviderPath + Write-Host "Files under $base (relative paths):" + tree --noreport $builtPath }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/copilot-setup-steps.yml(2 hunks).vscode/settings.json(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/copilot-setup-steps.yml
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (4)
.vscode/settings.json (1)
92-98: LGTM: Added cSpell words are appropriate for the workflow changesThe new dictionary terms align with the Linux/apt repo setup and GitHub Actions grouping terminology introduced in the workflow. No issues.
.github/workflows/copilot-setup-steps.yml (3)
32-65: Robust Git history handling for GitVersion looks goodThe shallow check with conditional fetch and exit-code validation is solid and defensive. This should keep GitVersion stable under Copilot branch strategies.
98-113: Resilient OMI install is goodThe conditional install with warnings if the package is missing or fails is appropriate for ubuntu-latest where omi may be unavailable. Nicely handled.
161-174: Idempotent .NET tools install: LGTMUsing update with fallback to install is the right pattern. Version constraint to 5.* is explicit and consistent with many GitVersion setups.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (4)
.github/workflows/copilot-setup-steps.yml (4)
224-267: Import the built module by manifest path; make DSC resource listing non-fatalAvoid importing an installed version by name; import the built manifest explicitly. Also, keep the DSC listing informational-only.
Write-Host "Importing built $env:MODULE_NAME module..." ./build.ps1 -Tasks 'noop' - Import-Module -Name $env:MODULE_NAME -Force + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) + $manifest = Join-Path -Path $builtPath -ChildPath "$env:MODULE_NAME.psd1" + if (-not (Test-Path -Path $manifest)) { + Write-Error "Module manifest not found at: $manifest" + exit 1 + } + Import-Module -FullyQualifiedName $manifest -Force # Verify module is loaded $module = Get-Module -Name $env:MODULE_NAME -ErrorAction SilentlyContinue if ($module) { @@ - $commands = @(Get-Command -Module $env:MODULE_NAME) - if ($commands.Count -gt 0) + $commands = @(Get-Command -Module $env:MODULE_NAME) + if ($commands.Count -gt 0) { Write-Host " Exported Commands: $($commands.Count)" Write-Host "`nAvailable Commands:" $commands | Select-Object Name, ModuleName | Format-Table -AutoSize } else { Write-Host 'No commands exported by the module.' } - $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) - Write-Host "`n Available class-based DSC Resources: $($dscResources.Count)`n" - $dscResources | Format-Table type, capabilities, implementedAs, requireAdapter -GroupBy kind -AutoSize + try { + $dscResources = @(dsc resource list --adapter Microsoft.DSC/PowerShell --output-format json | ConvertFrom-Json) + Write-Host "`n Available class-based DSC Resources: $($dscResources.Count)`n" + $dscResources | Format-Table type, capabilities, implementedAs, requireAdapter -GroupBy kind -AutoSize + } catch { + Write-Warning "Failed to list DSC resources: $($_.Exception.Message)" + } } else { Write-Error 'Failed to import module' exit 1 }
2-3: Parameterize MODULE_NAME via workflow_dispatch input (optional)Allow overriding the module name at dispatch time without editing the file; keep the current default for PR/push triggers.
env: - MODULE_NAME: 'SqlServerDsc' + MODULE_NAME: ${{ inputs.module_name || 'SqlServerDsc' }}And define the input under workflow_dispatch:
-on: + 'on': workflow_dispatch: + inputs: + module_name: + description: 'PowerShell module name to build/import' + required: false + default: 'SqlServerDsc' pull_request: paths: - '.github/workflows/copilot-setup-steps.yml' push: paths: - '.github/workflows/copilot-setup-steps.yml'
8-16: Quote the top-level ‘on’ key to satisfy YAML lintersYAMLlint flags unquoted “on” as a truthy literal. Quoting avoids noise without changing semantics.
-on: + 'on': workflow_dispatch: pull_request: paths: - '.github/workflows/copilot-setup-steps.yml' push: paths: - '.github/workflows/copilot-setup-steps.yml'
206-217: Use Join-Path and fix the “relative paths” comment by running tree from the built dirThe comment mentions stripping prefixes, but the script doesn’t. Use Push-Location and invoke tree on “.”; also prefer Join-Path for portability.
- $builtPath = "output/builtModule/$env:MODULE_NAME" + $builtPath = Join-Path -Path 'output' -ChildPath (Join-Path -Path 'builtModule' -ChildPath $env:MODULE_NAME) if (Test-Path -Path $builtPath) { Write-Host "Module built successfully at: $builtPath" - # Show files with paths relative to the built module base path using `tree` (run directly in PowerShell) - $base = (Resolve-Path -Path $builtPath).ProviderPath - Write-Host "Files under $base (relative paths):" - - # Run tree directly and strip leading './' from its output - tree --noreport $builtPath + # Show files with paths relative to the built module base path + Push-Location -Path $builtPath + try { + Write-Host "Files under $(Get-Location) (relative paths):" + tree --noreport . + } finally { + Pop-Location + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/copilot-setup-steps.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/copilot-setup-steps.yml
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (1)
.github/workflows/copilot-setup-steps.yml (1)
39-65: LGTM: Robust full-history fetch for GitVersionThe shallow/non-shallow detection and guarded fetch logic are solid and will give GitVersion what it needs. Good use of $LASTEXITCODE checks.
Pull Request (PR) description
.github/workflows/copilot-setup-steps.ymlMODULE_NAMEenv var and Linux‑friendly paths.This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is