Skip to content

FEAT: Software Factories#4111

Draft
iancooper wants to merge 2 commits into
masterfrom
software_factories
Draft

FEAT: Software Factories#4111
iancooper wants to merge 2 commits into
masterfrom
software_factories

Conversation

@iancooper

Copy link
Copy Markdown
Member

Description

This includes descriptions of how we create our gateway and Db implementations, so that we can create skills for agents that allow us have agents build new Inbox/Outbox implementations or new Messaging Gateways

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No application code in the PR — skipped Code Health checks.

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@claude

claude Bot commented May 1, 2026

Copy link
Copy Markdown

PR Review: FEAT: Software Factories

Thanks for this PR, Ian. The concept of agent-optimized "software factory" documentation is interesting and aligns well with the repo's direction. Since this is a draft, I'll provide detailed feedback to help get it to a mergeable state.


🚫 Blocker: Unresolved Merge Conflict in producers.md

docs/factories/transports/producers.md contains raw, unresolved git conflict markers:

<<<<<<<< HEAD:docs/factories/transports/producers.md
When the message has been sent, e need to mark it as dispatched in the Outbox.
...
========
...
>>>>>>>> master:docs/transports/producers.md

The master side also ends mid-sentence ("In the former case, you should hook up the callback to"), so both sides are incomplete. This must be resolved before merge — any agent or human reading this file will receive garbled, contradictory instructions.


🔴 High Priority Issues

1. Broken link: consmers.md in transports.md (line 29)

See [consumers](./consmers.md) for more information on how to implement.

consmers.md should be consumers.md. The file consumers.md does exist (though empty — see below). This typo is copied from docs/transports/transports.md, so both files share the bug. Worth fixing in both places.

2. Incorrect relative path in channelfactory.md

[IAmAChannelFactory](../../src/Paramore.Brighter/IAmAChannelFactory.cs)

From docs/factories/transports/, ../../src/ resolves to docs/src/, not the repo root. The path needs three levels up: ../../../src/Paramore.Brighter/IAmAChannelFactory.cs. The RabbitMQ example links on the same line correctly use ../../../.

3. consumers.md is completely empty

docs/factories/transports/consumers.md is 0 bytes, yet IAmAMessageConsumer is described in transports.md as a mandatory interface. An agent following the link will receive nothing useful. This file should be populated before the PR is marked ready for review.


🟡 Medium Priority Issues

4. Stub files: publication.md and subscription.md

Both files contain only a heading with no content. transports.md links to both. Shipping linked-but-empty files degrades the agent experience.

5. Boxes (Inbox/Outbox) documentation missing

The PR description says:

"descriptions of how we create our gateway and Db implementations, so that we can create skills for agents that allow us have agents build new Inbox/Outbox implementations"

No boxes/ subdirectory or related files were added. Is this intentional scope reduction, or planned for a follow-up?

6. Parallel doc trees without a sync strategy

docs/factories/transports/ is nearly identical to docs/transports/. This creates a maintenance burden: every future update to docs/transports/ must also be mirrored in docs/factories/transports/. The README.md doesn't explain the relationship between the two trees or how contributors should keep them in sync. Consider either:

  • Documenting the relationship and sync expectations explicitly, or
  • Having docs/factories/ link to or include content from docs/transports/ rather than duplicating it

🟢 Low Priority / Nits

Several grammar/typo issues (some copied from the original docs/transports/ files — worth fixing at source too):

File Line Issue
producers.md (HEAD side) 61 "e need to mark it""we need to mark it"
channels.md 7 "You MUST NOT to implement""You MUST NOT implement"
channels.md 25 "It send to a dead-letter queue""It sends to"
transports.md 11 Sentence ends with "for example" — the example filename is missing
transports.md 32 Missing space: `Channel`, which implements`IAmAChannel`
channelfactory.md 5 Mixed quote/backtick: 'IAmAMessageConsumer' ` — mismatched delimiters

Summary

The PR is appropriately marked as draft. The unresolved merge conflict is the only hard blocker; the rest is content completeness and quality. Once the conflicts are resolved and the empty/stub files are populated, this will be a solid foundation for the agent-skills workflow.

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review: FEAT: Software Factories (#4111)

Nice idea — capturing "how we build a transport" as agent-consumable docs is genuinely useful, and the MUST/SHOULD phrasing reads well for skill authoring. I checked the doc claims against the source and they're largely accurate (IAmAChannelFactory.CreateSyncChannel/CreateAsyncChannel/CreateAsyncChannelAsync, IAmAMessageProducer.{Publication,Span,Scheduler}, and ISupportPublishConfirmation all exist as described). A few things should be fixed before merge, though.

🔴 Blockers

  1. Unresolved merge-conflict markers committeddocs/factories/transports/producers.md contains raw conflict markers (<<<<<<<< HEAD:docs/factories/transports/producers.md / ======== / >>>>>>>> master:docs/transports/producers.md). This needs to be resolved to a single version. The "master" side also ends mid-sentence: "If the transport does not the latter case, ... you should hook up the callback to" — pick the HEAD wording (which reads cleanly) and drop the truncated fragment.

  2. Broken relative link in channelfactory.md: [`IAmAChannelFactory`](../../src/Paramore.Brighter/IAmAChannelFactory.cs) — from docs/factories/transports/ this resolves to docs/src/... (doesn't exist). Every other link in these files correctly uses ../../../src/.... Should be ../../../src/Paramore.Brighter/IAmAChannelFactory.cs.

  3. Broken link (typo) in transports.md: [consumers](./consmers.md)./consumers.md.

🟠 Content gaps

  1. Empty / stub files that are linked as "more detail":

    • consumers.md — 0 bytes, but channelfactory.md and transports.md both link to it "for more."
    • publication.md and subscription.md — header only, also linked "for more details."

    As-is these are dead ends for both humans and the agents this is meant to serve. Either fill them in or add a "TODO / stub" note so a reader (or skill) knows the content is pending rather than missing.

  2. Duplication of the existing docs/transports/ directorydocs/transports/ already exists with the same file set (and the same empty consumers.md, stub publication.md/subscription.md). This PR adds a near-identical docs/factories/transports/. The conflict marker even references docs/transports/producers.md as the "master" path. Worth clarifying the intent: is docs/factories/ meant to supersede docs/transports/ (then remove the old one), or coexist (then they'll drift)? Two copies of the same guidance is a maintenance hazard.

🟡 Accuracy / polish

  1. Factual: channels.md says Stop posts a WM-QUIT message — the actual constant is MT_QUIT (MessageType.MT_QUIT, see MessageHeader.cs). WM-QUIT is the Win32 message; using it here is misleading.

  2. Copy-paste slips in transports.md: IAmAMessageConsumer and IAmAChannel are each described as having "derived interfaces for sync and async producers" — should read consumers / channels respectively.

  3. Typos / grammar:

    • producers.md: "e need to mark it as dispatched" → "we need".
    • channels.md: "You MUST NOT to implement" → "You MUST NOT implement".
    • channelfactory.md: backtick mismatch — IAmAMessageConsumer' (closing ' instead of a backtick).
    • transports.md: sentence trails off — "...so docker-compose-X.yaml where X is the name of the middleware, for example These tests use I/O..." (missing example, run-on into next sentence). Also "use I/O to the middleware over test doubles" reads oddly — did you mean "rather than test doubles"?
  4. Minor: several files are missing a trailing newline (channelfactory.md, channels.md, producers.md).

None of this is architectural — the structure and the normative language are good. Resolving the conflict markers (1), fixing the two broken links (2, 3), and deciding what to do about the duplicate directory (5) are the ones I'd treat as merge-blocking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant