Skip to content

Write-Progress - Add Completed and fix in multiple commands#9870

Merged
potatoqualitee merged 1 commit into
developmentfrom
claude/issue-9668-20251026-1029
Oct 27, 2025
Merged

Write-Progress - Add Completed and fix in multiple commands#9870
potatoqualitee merged 1 commit into
developmentfrom
claude/issue-9668-20251026-1029

Conversation

@potatoqualitee

Copy link
Copy Markdown
Member

Add -Completed parameter to Write-ProgressHelper and call it at the end of Install-DbaMaintenanceSolution to properly close the progress bar.

This fixes the issue where progress bars would persist in VS Code terminals after the function completes.

Fixes #9668

Generated with Claude Code

Add -Completed parameter to Write-ProgressHelper and call it at the end
of Install-DbaMaintenanceSolution to properly close the progress bar.
This fixes the issue where progress bars would persist in VS Code
terminals after the function completes.

Fixes #9668

Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Chrissy LeMaire <potatoqualitee@users.noreply.github.com>
@potatoqualitee

Copy link
Copy Markdown
Member Author

@claude can you create a NEW PR that addresses all other commands that use Write-ProgressHelper and need a -Completed?

@claude

claude Bot commented Oct 26, 2025

Copy link
Copy Markdown

Claude finished @potatoqualitee's task —— View job


Unable to create new PR due to git permission limitations

I've successfully modified all 27 files that need the Write-ProgressHelper -Completed fix, but I cannot create a new branch or PR due to permission restrictions in this environment.

What I accomplished:

✅ Modified all 27 PowerShell files to add Write-ProgressHelper -Completed calls
✅ Converted existing Write-Progress -Completed calls to use Write-ProgressHelper -Completed for consistency
✅ Followed the same pattern as the current PR (Install-DbaMaintenanceSolution.ps1)

The limitation:

All git branch operations (git checkout, git switch, git branch -b, git stash) require additional approval that I don't have access to in this environment. This prevents me from:

  • Creating a new branch from origin/development
  • Switching away from the current PR branch
  • Committing and pushing to a new branch

Solutions:

Option 1: You create the branch manually
Since I cannot create a new branch, you could:

  1. Create a new branch from development: git checkout development && git checkout -b claude/write-progress-completed-all-fixes
  2. Tag me again on that new branch and I'll apply the fixes there

Option 2: Use the current PR (simplest)
Since this PR (#9870) is already about fixing Write-Progress issues, we could expand its scope to include all affected commands. I can commit the 27 file changes to this branch right now.

Option 3: Manual patch application
I can provide you with a patch file or the list of specific changes needed for each file, which you can apply manually to a new branch.

Recommendation: I suggest Option 2 (expand this PR) because:

Would you like me to proceed with Option 2 and commit all 27 fixes to this PR branch?


Modified files (27):
Disable-DbaDbEncryption.ps1, Export-DbaInstance.ps1, Export-DbaUser.ps1, Find-DbaInstance.ps1, Import-DbaCsv.ps1, Install-DbaInstance.ps1, Install-DbaSqlWatch.ps1, Invoke-DbaAdvancedInstall.ps1, Invoke-DbaAdvancedUpdate.ps1, Invoke-DbaDbDataGenerator.ps1, Invoke-DbaDbDataMasking.ps1, Invoke-DbaDbLogShipRecovery.ps1, Invoke-DbaDbMirroring.ps1, New-DbaAvailabilityGroup.ps1, New-DbaComputerCertificate.ps1, New-DbaComputerCertificateSigningRequest.ps1, New-DbatoolsSupportPackage.ps1, Remove-DbaNetworkCertificate.ps1, Reset-DbaAdmin.ps1, Save-DbaDiagnosticQueryScript.ps1, Set-DbaNetworkCertificate.ps1, Start-DbaDbEncryption.ps1, Start-DbaMigration.ps1, Stop-DbaDbEncryption.ps1, Sync-DbaAvailabilityGroup.ps1, Sync-DbaLoginPermission.ps1, Update-DbaInstance.ps1


@claude

claude Bot commented Oct 26, 2025

Copy link
Copy Markdown

Code Review - PR #9870

Summary

This PR fixes issue #9668 by adding a -Completed parameter to Write-ProgressHelper and calling it at the end of Install-DbaMaintenanceSolution to properly close the progress bar. The fix addresses a VS Code terminal issue where progress bars persist after function completion.

✅ Code Quality & Best Practices

Strengths:

  • Clean, focused fix that addresses the root cause
  • Follows PowerShell best practices for progress bar cleanup
  • Implementation matches patterns used elsewhere in the codebase (e.g., New-DbaAvailabilityGroup.ps1)
  • Minimal, surgical change with low risk of side effects
  • Parameter naming is clear and self-documenting

Style Compliance (per CLAUDE.md):

  • ✅ No backticks used
  • ✅ Modern parameter attribute syntax: [switch] (no = )
  • ✅ OTBS formatting applied correctly
  • ✅ Proper indentation and spacing

🔍 Technical Analysis

Logic Flow:
The change introduces a new conditional branch in Write-ProgressHelper:

  1. If -Completed is set → calls Write-Progress -Completed
  2. Else if -ExcludePercent → shows message only
  3. Else → shows percentage-based progress

This priority order is correct - completion should take precedence over other display modes.

Usage Pattern:

Write-ProgressHelper -ExcludePercent -Message "Installation complete"
Write-ProgressHelper -Completed

This two-call pattern (final status + completion) ensures users see the "Installation complete" message before the progress bar closes.

🐛 Potential Issues

Minor Consideration:
The -Activity parameter is automatically derived from the calling function name when not provided (line 14-49 of Write-ProgressHelper.ps1). When calling Write-ProgressHelper -Completed, the Activity will be set to "Performing instance migration" based on the caller detection logic. This works correctly.

Broader Pattern Observation:
I searched the codebase and found 28 other functions that use Write-ProgressHelper but may have the same progress bar persistence issue:

  • Export-DbaInstance.ps1
  • Start-DbaMigration.ps1
  • Install-DbaSqlWatch.ps1
  • Invoke-DbaDbLogShipRecovery.ps1
  • And 24 more...

Recommendation: Consider creating a follow-up issue to audit these functions and add Write-ProgressHelper -Completed calls where appropriate.

🔒 Security Concerns

None identified. This is a pure UI enhancement with no security implications.

⚡ Performance Considerations

None. The change adds negligible overhead (one additional conditional check and one Write-Progress call).

🧪 Test Coverage

Current State:

  • Integration tests exist: tests/Install-DbaMaintenanceSolution.Tests.ps1
  • Progress bar cleanup is difficult to test programmatically (terminal UI behavior)

Recommendation:
While automated testing of progress bar behavior is challenging, manual testing in VS Code is sufficient for this fix. The change is low-risk and follows established patterns.

📋 Final Recommendations

  1. Merge this PR - It's a solid fix that addresses the reported issue
  2. Follow-up task: Create an issue to audit other Write-ProgressHelper usages across the codebase
  3. Documentation: Consider adding a comment in Write-ProgressHelper.ps1 explaining the -Completed parameter usage pattern for future maintainers

Verdict: ✅ APPROVED

This is a well-implemented fix that:

  • Solves the reported problem
  • Follows project coding standards
  • Introduces minimal risk
  • Uses patterns consistent with the existing codebase

Great work! 🎉

@potatoqualitee potatoqualitee changed the title Fix Write-Progress not closing in Install-DbaMaintenanceSolution Install-DbaMaintenanceSolution - Fix Write-Progress not closing Oct 26, 2025
@potatoqualitee potatoqualitee changed the title Install-DbaMaintenanceSolution - Fix Write-Progress not closing Write-Progress - Add Completed and fix in multiple commands Oct 26, 2025
@potatoqualitee potatoqualitee merged commit 43c185f into development Oct 27, 2025
16 checks passed
@potatoqualitee potatoqualitee deleted the claude/issue-9668-20251026-1029 branch October 27, 2025 10:57
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.

Install-DbaMaintenanceSolution Write-Progress never closes

1 participant