Skip to content

REVIEW 2.1

Brian Lehnen edited this page Apr 8, 2026 · 1 revision

phase: 02-core-usage plan: "2.1" wave: 2 reviewed_by: reviewer-agent date: 2026-04-08 verdict: REQUEST CHANGES

Review 2.1 -- Consumer pages + Linq→Method renames

Stage 1: Spec Compliance

Verdict: FAIL


Task 1: Fix Consumer.md -- verify Start() signature and logging API

  • Status: PASS
  • Evidence: /mnt/f/git/dotnetworkqueue.wiki/Consumer.md
    • queue.Start<SimpleMessage>(HandleMessages, notifications) present (line 32)
    • ConsumerQueueNotifications constructed with 6 callbacks (lines 24-30); grep -c returns 2 (code + prose at line 14)
    • Logging section uses notifications.Log.LogDebug, LogInformation, LogWarning, LogError (lines 82-85); grep returns 4
    • IWorkerNotification properties table includes Log as ILogger, Metrics as IMetrics, Tracer as ActivitySource (lines 67-75)
    • Zero occurrences of DotNetWorkQueue.Logging.LogLevel or Log.Log( pattern
  • Notes: One deviation from the spec: the plan requires a samples link at the bottom of the "Creating the queue" section. grep -n "samples" Consumer.md returns no output -- the link is absent. However, the plan marks this as an "if not present, add" action; the page was otherwise already correct. This is a minor omission but the done criteria do not explicitly gate on the link's presence, so Task 1 is marked PASS with a finding raised below.

Task 2: Fix ConsumerAsync.md -- add ConsumerQueueNotifications and fix logging

  • Status: PASS
  • Evidence: /mnt/f/git/dotnetworkqueue.wiki/ConsumerAsync.md
    • CreateConsumerQueueScheduler and SchedulerContainer both present (grep returns 3)
    • ConsumerQueueNotifications with 6 callbacks (lines 33-39); grep returns 2
    • queue.Start<SimpleMessage>(Handle, notifications) (line 40)
    • Cancellation example uses notifications.Log.LogDebug("Cancel has been requested - aborting") (line 66)
    • Scheduler dispose note present (lines 81-85)
    • Scheduler-based note / distinction paragraph present (lines 5-6)
    • Samples link present: > For a complete working example, see [SQLServerConsumerAsync](...) (line 89)
    • Zero occurrences of broken logging pattern
  • Notes: All 7 action items from the spec are satisfied.

Task 3: Rename Linq pages to Method pages and delete old files

  • Status: FAIL
  • Evidence:

A) ProducerMethod.md -- /mnt/f/git/dotnetworkqueue.wiki/ProducerMethod.md

  • File exists. Title is "Producing Method Expressions" (line 1). PASS.
  • LinqExpressionToRun / "dynamic statement" section absent; grep returns 0. PASS.
  • "Unlike the typed-message producer" used (line 13). PASS.
  • CreateMethodProducer present (line 19). PASS.
  • Samples link present (line 68). PASS.
  • No issues.

B) ConsumerMethod.md -- /mnt/f/git/dotnetworkqueue.wiki/ConsumerMethod.md

  • File exists. Title is "Consuming Method Expressions" (line 1). PASS.
  • Link to ConsumerMethodAsync in opening line (line 3). PASS.
  • ConsumerQueueNotifications present in code sample (line 21). PASS.
  • "method expression" / "lambda expression" terminology used. PASS.
  • Logging uses logger.LogError, logger.LogWarning, logger.LogInformation (lines 22-27). PASS.
  • Samples link present (line 73). PASS.
  • FAIL -- ConsumerQueueNotifications grep count is 1 (code only), not >= 2. The spec done criteria for Task 3 do not explicitly require a count >= 2 on ConsumerMethod.md, but the plan (Task 3B) states "Verify ConsumerQueueNotifications is present in the code sample" -- which it is. Count of 1 is acceptable here.
  • FAIL -- CreateConsumerMethodQueueScheduler is used (line 17) but factory is referenced as a parameter without being constructed or passed in scope. The code sample passes factory to CreateConsumerMethodQueueScheduler but never shows factory being created (no SchedulerContainer / CreateTaskFactory setup). The spec says "The code sample already uses CreateConsumerMethodQueueScheduler and queue.Start(notifications) -- this is correct" -- however the sample is incomplete: it omits the scheduler setup that its own parameter requires, making the sample misleading. This is an introduced defect relative to the source file (ConsumerLinq.md did not show a factory parameter).

C) ConsumerMethodAsync.md -- /mnt/f/git/dotnetworkqueue.wiki/ConsumerMethodAsync.md

  • File exists. PASS.
  • FAIL -- Title is "Consuming messages" (line 1), not "Consuming Method Expressions (Async)". The spec requires: Title: "Consuming Method Expressions (Async)". The page retains the old generic title.
  • Link to ConsumerMethod (not ConsumerLinq) in opening line (line 3). PASS.
  • ConsumerQueueNotifications present in code (line 30). PASS.
  • factory.Scheduler (capital S) used correctly on lines 21-23. PASS.
  • "When your method expression finishes" present (line 48). PASS.
  • Samples link present (line 90). PASS.
  • No LinqExpressionToRun or dynamic LINQ references. PASS.

D) Old file deletion:

  • ProducerLinq.md: does not exist. PASS.
  • ConsumerLinq.md: does not exist. PASS.
  • ConsumerLinqAsync.md: does not exist. PASS.

Task 3 done criteria failure: The spec requires ConsumerMethodAsync.md to have title "Consuming Method Expressions (Async)". The actual title is "Consuming messages". This is an explicit, verifiable done-criteria failure.


Stage 2: Code Quality

Not performed -- Stage 1 FAIL.


Findings Summary

Critical

  • ConsumerMethodAsync.md has wrong title -- /mnt/f/git/dotnetworkqueue.wiki/ConsumerMethodAsync.md, line 1
    • Actual: #### Consuming messages
    • Required by spec (Task 3C done criteria): #### Consuming Method Expressions (Async)
    • Remediation: Change line 1 from #### Consuming messages to #### Consuming Method Expressions (Async).

Important

  • ConsumerMethod.md code sample references undefined factory variable -- /mnt/f/git/dotnetworkqueue.wiki/ConsumerMethod.md, line 17

    • queueContainer.CreateConsumerMethodQueueScheduler(queueConnection, factory) passes factory but no scheduler/factory setup (SchedulerContainer, CreateTaskFactory) is shown. A reader cannot reproduce this from the sample alone. ConsumerMethodAsync.md correctly shows the full scheduler setup.
    • Remediation: Either add the scheduler setup block (as shown in ConsumerMethodAsync.md lines 16-23) before the queueContainer block, or remove factory from the call and show the parameterless overload if one exists.
  • Consumer.md missing samples link -- /mnt/f/git/dotnetworkqueue.wiki/Consumer.md

    • The plan (Task 1 action, final bullet) requires: > For a complete working example, see [SQLServerConsumer](https://github.com/blehnen/DotNetWorkQueue.Samples) in the samples repository. This link is absent from the file.
    • Remediation: Add the samples link at the bottom of the "Creating the queue" section (after the first code block, before the cancellation section).

Suggestions

  • ProducerMethod.md indentation inconsistency -- /mnt/f/git/dotnetworkqueue.wiki/ProducerMethod.md, lines 19-22
    • The using block uses mixed tabs inside the code fence. Minor cosmetic issue; not a spec violation.

Summary

Verdict: REQUEST CHANGES

Stage 1 fails on one explicit done-criteria item: ConsumerMethodAsync.md retains the title "Consuming messages" instead of the required "Consuming Method Expressions (Async)". Additionally, ConsumerMethod.md has an undefined factory variable in its code sample that will confuse readers. Fix the title (one-line change) and the ConsumerMethod.md code sample before this wave can be marked complete.

Critical: 1 | Important: 2 | Suggestions: 1

Clone this wiki locally