Skip to content

Address code review feedback: fix resource disposal and code quality issues#2

Merged
BrettKinny merged 3 commits intoclaude/build-opcscope-client-Y89wJfrom
copilot/sub-pr-1
Jan 5, 2026
Merged

Address code review feedback: fix resource disposal and code quality issues#2
BrettKinny merged 3 commits intoclaude/build-opcscope-client-Y89wJfrom
copilot/sub-pr-1

Conversation

Copy link
Copy Markdown

Copilot AI commented Jan 5, 2026

Applies actionable feedback from the automated code review on PR #1.

Resource Management

  • Added using var to SaveDialog in CSV export to ensure disposal
  • Fixed empty catch block in SubscriptionManager.Dispose() with diagnostic logging

Code Quality

  • Extracted AllocatePort() static method to eliminate static field mutation from instance context
  • Replaced implicit foreach filtering with explicit LINQ Where() in MonitoredItem_Notification
  • Fixed KeyValuePair default check: matchingKvp.Value != null!matchingKvp.Equals(default(...))
  • Removed unused JavaScript variables (nodesets, counterVariable, etc.)

API Correctness

  • Added ApplicationConfiguration parameter to CoreClientUtils.SelectEndpoint() call
  • Reordered CertificateValidator initialization before Validate() to set up event handlers first

Infrastructure

  • Updated NuGet.Config to use nuget.org source for CI/CD compatibility

Not Applied

Obsolete method warnings were not addressed - these methods are not deprecated in the current OPC Foundation SDK (v1.5+), and the build produces zero warnings.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Jan 5, 2026
Copilot AI and others added 2 commits January 5, 2026 08:02
…API usage

Co-authored-by: BrettKinny <14231653+BrettKinny@users.noreply.github.com>
Co-authored-by: BrettKinny <14231653+BrettKinny@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Jan 5, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • crl.entrust.net
    • Triggering command: /usr/bin/dotnet dotnet restore (dns block)
    • Triggering command: /usr/bin/dotnet dotnet restore tests/OpcScope.Tests/OpcScope.Tests.csproj (dns block)
    • Triggering command: /usr/bin/dotnet dotnet restore --no-dependencies /home/REDACTED/work/OpcScope/OpcScope/tests/OpcScope.Tests/OpcScope.Tests.csproj --packages /home/REDACTED/work/OpcScope/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/OpcScope/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/OpcScope/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot (dns block)
  • ocsp.entrust.net
    • Triggering command: /usr/bin/dotnet dotnet restore (dns block)
    • Triggering command: /usr/bin/dotnet dotnet restore tests/OpcScope.Tests/OpcScope.Tests.csproj (dns block)
    • Triggering command: /usr/bin/dotnet dotnet restore --no-dependencies /home/REDACTED/work/OpcScope/OpcScope/tests/OpcScope.Tests/OpcScope.Tests.csproj --packages /home/REDACTED/work/OpcScope/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/OpcScope/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/OpcScope/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Implement OPCScope terminal-based OPC UA client Address code review feedback: fix resource disposal and code quality issues Jan 5, 2026
Copilot AI requested a review from BrettKinny January 5, 2026 08:09
@BrettKinny BrettKinny marked this pull request as ready for review January 5, 2026 08:18
@BrettKinny BrettKinny merged commit 614709f into claude/build-opcscope-client-Y89wJ Jan 5, 2026
@BrettKinny BrettKinny deleted the copilot/sub-pr-1 branch January 5, 2026 08:18
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