Upgrade newest standards by Chris-Wolfgang · Pull Request #67 · Chris-Wolfgang/ETL-Abstractions

Chris-Wolfgang · 2026-03-10T00:05:59Z

Description

Modified code and examples to comply with code scan results
Fixes/Complete # (issue)

…, and ExtractorBase Co-authored-by: Chris-Wolfgang <210299580+Chris-Wolfgang@users.noreply.github.com>

Co-authored-by: Chris-Wolfgang <210299580+Chris-Wolfgang@users.noreply.github.com>

…tractor.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…Transformer.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…sis-errors

…s-errors Fix static analysis errors in Example6-ReducingDuplicateCode (Net8.0)

Chris-Wolfgang · 2026-03-10T00:17:28Z

@copilot

PS C:\Source\GitHub\ETL-Framework\ETL-Abstractions> del .\Wolfgang.Etl.Abstractions.sln
PS C:\Source\GitHub\ETL-Framework\ETL-Abstractions> dotnet build -c:release
C:\Source\GitHub\ETL-Framework\ETL-Abstractions\tests\Wolfgang.Etl.Abstractions.Tests.Unit\Wolfgang.Etl.Abstractions.Tests.Unit.csproj : error NU1701: Warning As Error: Package 'xunit.runner.visualstudio 2.8.2' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8, .NETFramework,Version=v4.8.1' instead of the project target framework '.NETCoreApp,Version=v3.1'. This package may not be fully compatible with your project.

Restore failed with 1 error(s) in 17.4s

Copilot · 2026-03-10T00:17:34Z

@Chris-Wolfgang I've opened a new pull request, #68, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Updates core ETL abstraction base classes and the .NET 8 example to address code scanning/analyzer findings (exception types/docs, argument validation, and API usage patterns), plus a small repo hygiene tweak.

Changes:

Updated XML docs/guards to prefer ArgumentOutOfRangeException and added null-argument exception documentation in base classes.
Adjusted MaximumItemCount validation (now >= 1 for Loader/Transformer) and applied small style tweaks (e.g., named state: in Timer ctor).
Refactored Example6 types into separate files and added ConfigureAwait(false) in several awaits.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
src/Wolfgang.Etl.Abstractions/TransformerBase.cs	Doc/guard updates; `MaximumItemCount` validation tweak; timer callback arg naming.
src/Wolfgang.Etl.Abstractions/LoaderBase.cs	Doc/guard updates; `MaximumItemCount` validation tweak; timer callback arg naming.
src/Wolfgang.Etl.Abstractions/ExtractorBase.cs	Doc/guard updates; timer callback arg naming.
examples/Net8.0/Example6-ReducingDuplicateCode/Program.cs	Added `ConfigureAwait(false)`; moved nested helper types out.
examples/Net8.0/Example6-ReducingDuplicateCode/EtlProgress.cs	New file for `EtlProgress` record.
examples/Net8.0/Example6-ReducingDuplicateCode/ConsoleColors.cs	New file for `ConsoleColors`.
examples/Net8.0/Example6-ReducingDuplicateCode/ETL/IntToStringTransformer.cs	Updated null guards/arg naming; timer + delay changes.
examples/Net8.0/Example6-ReducingDuplicateCode/ETL/FibonacciExtractor.cs	Updated null guards/arg naming; timer + delay changes.
examples/Net8.0/Example6-ReducingDuplicateCode/ETL/ConsoleLoader.cs	Updated null guards/arg naming; timer + delay changes.
.gitignore	Ignored `.nuget/nuget.exe`.

Comments suppressed due to low confidence (3)

src/Wolfgang.Etl.Abstractions/ExtractorBase.cs:82

CurrentSkippedItemCount’s setter never assigns the backing field when the value is valid, so the property can’t be updated. Assign _currentSkippedItemCount = value after validation (or remove the setter if it isn’t intended to be set).

        get => _currentSkippedItemCount;
        protected set
        {
            if (value < 0)
            {
                throw new ArgumentOutOfRangeException(nameof(value), "value cannot be less than 0.");
            }

src/Wolfgang.Etl.Abstractions/LoaderBase.cs:238

LoadAsync(items, progress) returns the worker Task without awaiting it, but the progress Timer is in a using var, so it will be disposed immediately when the method returns, preventing progress from being reported. Make the method async and await the worker (or otherwise keep the timer alive until the load completes).

        using var timer = new Timer
        (
            _ => progress.Report(CreateProgressReport()),
            state: null,
            TimeSpan.Zero,
            TimeSpan.FromMilliseconds(ReportingInterval)
        );

        return LoadWorkerAsync(items, CancellationToken.None);

src/Wolfgang.Etl.Abstractions/ExtractorBase.cs:273

Same timer-lifetime issue as the other ExtractAsync overload: disposing the Timer in the method scope means it will be gone before the returned IAsyncEnumerable is enumerated, so progress reporting won’t occur. Ensure the timer lifetime is tied to enumeration (e.g., via an async iterator wrapper).

        using var timer = new Timer
        (
            _ => progress.Report(CreateProgressReport()),
            state: null,
            TimeSpan.Zero,
                
            TimeSpan.FromMilliseconds(ReportingInterval)
        );

Copilot · 2026-03-10T02:16:54Z

                _ => progress?.Report(new EtlProgress(Volatile.Read(ref count))),
-                null,
+                state: null,
                TimeSpan.Zero,
                TimeSpan.FromMilliseconds(_progressInterval) // Use the configured progress interval
-            );
+            ).ConfigureAwait(false);


System.Threading.Timer is IDisposable (not IAsyncDisposable) and the constructor doesn’t return a task, so await using and the trailing .ConfigureAwait(false) on the timer creation won’t compile. Use a normal using var for the timer (or switch to PeriodicTimer with an async loop) and keep it scoped to the async iterator so it’s disposed when enumeration completes.

Copilot · 2026-03-10T02:16:54Z

        using var timer = new Timer
        (
            _ => progress.Report(CreateProgressReport()),
-            null,
+            state: null,
            TimeSpan.Zero,
            TimeSpan.FromMilliseconds(ReportingInterval)
        );


The Timer is created in a method that returns a deferred IAsyncEnumerable. Because it’s in a using var, it will be disposed as soon as TransformAsync returns (before enumeration starts), so progress reporting won’t run. Wrap the worker enumeration in an async iterator (or otherwise tie timer lifetime to enumeration) so the timer lives until the sequence completes/cancels.

Copilot · 2026-03-10T02:16:55Z

        using var timer = new Timer
        (
            _ => progress.Report(CreateProgressReport()),
-            null,
+            state: null,
            TimeSpan.Zero,
            TimeSpan.FromMilliseconds(ReportingInterval)
        );


Same timer-lifetime issue as the other TransformAsync overload: returning a deferred IAsyncEnumerable while the timer is in a using var disposes it immediately, preventing progress reports. Ensure the timer is disposed only after enumeration completes (e.g., by wrapping the worker in an async iterator that owns the timer).

Copilot · 2026-03-10T02:16:55Z

                Console.WriteLine($"Transforming integer {item} to string.");
-                await Task.Delay(50); // Simulate some delay for transformation
+                await Task.Delay(50, CancellationToken.None).ConfigureAwait(false); // Simulate some delay for transformation
                yield return item.ToString();


This delay is using CancellationToken.None, which makes cancellation less responsive (the iterator won’t observe cancellation until after the delay completes). Pass the method’s token to Task.Delay so cancellation can interrupt the delay promptly.

Copilot · 2026-03-10T02:16:55Z

        using var timer = new Timer
        (
            _ => progress.Report(CreateProgressReport()),
-            null,
+            state: null,
            TimeSpan.Zero,
            TimeSpan.FromMilliseconds(ReportingInterval)
        );


ExtractAsync(progress) returns a deferred IAsyncEnumerable, but the progress Timer is in a using var, so it will be disposed as soon as the method returns (before enumeration starts). Wrap the worker enumeration in an async iterator that owns the timer (or otherwise keep the timer alive for the duration of enumeration) so progress reports are actually emitted.

Copilot · 2026-03-10T02:16:56Z

        using var timer = new Timer
        (
            _ => progress.Report(CreateProgressReport()),
-            null,
+            state: null,
            TimeSpan.Zero,
            TimeSpan.FromMilliseconds(ReportingInterval)
        );


Same timer-lifetime issue as the other LoadAsync overload: disposing the Timer via using var before the returned Task completes prevents progress reporting. Ensure the timer is disposed only after LoadWorkerAsync finishes/cancels (e.g., by making this method async and awaiting the worker).

Copilot · 2026-03-10T02:16:56Z

    /// <summary>
    /// Gets the current number of records skipped
    /// </summary>
+    /// <exception cref="ArgumentOutOfRangeException">Value cannot be less than 0</exception>
    public int CurrentSkippedItemCount
    {
        get => _currentSkippedItemCount;


CurrentSkippedItemCount has a backing field and a protected set, but the setter body never assigns _currentSkippedItemCount when the value is valid (it only throws for negatives). That means derived classes can’t actually update this property via the setter. Assign the backing field after validation (or remove the setter if it’s intentionally not settable).

Copilot · 2026-03-10T02:16:56Z

    /// <summary>
    /// Gets the current number of records skipped
    /// </summary>
+    /// <exception cref="ArgumentOutOfRangeException">Value cannot be less than 0</exception>
    public int CurrentSkippedItemCount
    {
        get => _currentSkippedItemCount;


CurrentSkippedItemCount exposes a protected set, but the setter implementation never assigns _currentSkippedItemCount when the value is valid (it only throws for negatives). This makes the property effectively write-only via IncrementCurrentSkippedItemCount and can break derived implementations that set the property directly. Assign the backing field after validation (or remove the setter if that’s the intent).

Copilot · 2026-03-10T02:16:57Z

+            if (value < 1)
            {
-                throw new ArgumentOutOfRangeException(nameof(value), "Maximum item count cannot be less than 0.");
+                throw new ArgumentOutOfRangeException(nameof(value), "Maximum item count cannot be less than 1.");
            }
            _maximumItemCount = value;


This changes MaximumItemCount validation to disallow 0, while ExtractorBase.MaximumItemCount still allows 0. That makes the base-class APIs inconsistent and can be a breaking behavior change for consumers. Consider aligning all three base classes on the same allowed range (either allow 0 everywhere or disallow it everywhere) and update the docs/tests accordingly.

Copilot · 2026-03-10T02:16:57Z

+            if (value < 1)
            {
-                throw new ArgumentOutOfRangeException(nameof(value), "Maximum item count cannot be less than 0.");
+                throw new ArgumentOutOfRangeException(nameof(value), "Maximum item count cannot be less than 1.");
            }
            _maximumItemCount = value;


This changes MaximumItemCount validation to disallow 0, but ExtractorBase.MaximumItemCount still allows 0. That inconsistency in the public base-class API can surprise consumers. Consider aligning the allowed range across Extractor/Transformer/Loader base classes and adjusting docs/tests to match.

Copilot AI and others added 11 commits March 9, 2026 22:39

Initial plan

88e0d98

Fix MA0003 and RCS1140 analyzer errors in TransformerBase, LoaderBase…

2aae2d0

…, and ExtractorBase Co-authored-by: Chris-Wolfgang <210299580+Chris-Wolfgang@users.noreply.github.com>

Remove accidentally committed .nuget/nuget.exe and add to .gitignore

be95cfb

Co-authored-by: Chris-Wolfgang <210299580+Chris-Wolfgang@users.noreply.github.com>

Change minimum value for maximum item count to 1

7ff17cf

Change minimum value for maximum item count to 1

e0a1802

Fix static analysis errors in Example6-ReducingDuplicateCode (Net8.0)

24e6b59

Co-authored-by: Chris-Wolfgang <210299580+Chris-Wolfgang@users.noreply.github.com>

Remove accidentally committed nuget.exe binary

f1e2e8b

Co-authored-by: Chris-Wolfgang <210299580+Chris-Wolfgang@users.noreply.github.com>

Update examples/Net8.0/Example6-ReducingDuplicateCode/ETL/FibonacciEx…

574f696

…tractor.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update examples/Net8.0/Example6-ReducingDuplicateCode/ETL/IntToString…

f2bbb0d

…Transformer.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'Upgrade-newest-standards' into copilot/fix-static-analy…

a7f4589

…sis-errors

Merge pull request #66 from Chris-Wolfgang/copilot/fix-static-analysi…

22e436e

…s-errors Fix static analysis errors in Example6-ReducingDuplicateCode (Net8.0)

Copilot AI mentioned this pull request Mar 10, 2026

Fix NU1701 release build failure for xunit.runner.visualstudio on netcoreapp3.1 #68

Closed

9 tasks

Chris-Wolfgang marked this pull request as ready for review March 10, 2026 02:12

Copilot AI review requested due to automatic review settings March 10, 2026 02:12

Chris-Wolfgang merged commit f9d19d3 into main Mar 10, 2026
7 of 9 checks passed

Chris-Wolfgang deleted the Upgrade-newest-standards branch March 10, 2026 02:12

Copilot started reviewing on behalf of Chris-Wolfgang March 10, 2026 02:12 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

dependabot Bot mentioned this pull request Mar 14, 2026

Bump the dotnet-dependencies group with 8 updates Chris-Wolfgang/ETL-Test-Kit#5

Closed

Conversation

Chris-Wolfgang commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Chris-Wolfgang commented Mar 10, 2026

Uh oh!

Copilot AI commented Mar 10, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Chris-Wolfgang commented Mar 10, 2026 •

edited

Loading