Skip to content

Conversation

@holytshirt
Copy link
Member

Summary

  • Enhanced README.md with improved documentation structure and usage instructions
  • Updated CONTRIBUTING.md with detailed build & test instructions, Docker Compose guidance, and project structure overview
  • Fixed documentation typos ('Licence' spelling, 'ChannelAsync' typo)
  • Corrected link to CONTRIBUTING.md in Copilot instructions

Changes

  • README.md: Expanded documentation with better onboarding guidance
  • CONTRIBUTING.md: Added Build & Test section, Docker Compose usage tables, project structure breakdown
  • docs/transports/channelfactory.md: Fixed typo
  • .agent_instructions/documentation.md: Updated licence comment formatting
  • .github/copilot-instructions.md: Fixed link reference

holytshirt and others added 6 commits January 31, 2026 13:56
…andards

- Add Prerequisites section to README (targets .NET 8-10, C# 12-14)
- Add "What's Next?" navigation section after basic Quick Start
- Add Table of Contents to CONTRIBUTING.md for better navigation
- Add "First Time Contributing?" onboarding section with quick setup guide
- Fix license header documentation to use correct format (#region Licence)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Copy link

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

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
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@claude
Copy link

claude bot commented Jan 31, 2026

PR Review: Documentation Enhancements ✅

This is an excellent documentation improvement PR! The changes significantly enhance the onboarding experience and make the project more accessible to new contributors. Here's my detailed review:


✅ Strengths

1. README.md Improvements

  • Excellent structure: The new README follows modern best practices with clear sections, badges, and progressive disclosure
  • Quick Start section: The step-by-step examples with code snippets are very helpful for new users
  • Multiple use cases: Good coverage of in-process and out-of-process messaging patterns
  • Better discoverability: Links to documentation, supported transports, and NuGet packages

2. CONTRIBUTING.md Enhancements

  • "First Time Contributing?" section: Great addition! Lowers the barrier to entry
  • Docker Compose guidance: The table with specific compose files is very practical
  • Table of Contents: Makes the long document navigable
  • Build & Test section: Clear instructions with code examples

3. Consistency Fixes

  • License header correction: Fixed the important detail about British spelling (Licence not License) and proper formatting (#region without space)
  • Link corrections: Fixed .github/copilot-instructions.md relative path
  • Typo fix: Corrected ChanngeAsyncChannelAsync in channelfactory.md:930

🔍 Minor Issues & Suggestions

1. Duplicate Content in CONTRIBUTING.md

Location: CONTRIBUTING.md:239-247

There's a duplicate paragraph about TDD workflow:

- Where possible, avoid writing tests after.
  - This will not give you scope control - only writing the code required by tests.
    - You should only write the code necessary for a test to pass; do not write speculative code.
  - It will not push you to focus on design of your classes for behavior.
    - Pay attention to the usability of your class and method; it should be self-describing.
- Where possible, avoid writing tests after.
  - This will not give you scope control - only writing the code required by tests.

Recommendation: Remove the duplicate (lines 240-244).

2. File Rename Notice

Location: CONTRIBUTING.md (file rename from Contributing.md to CONTRIBUTING.md)

The rename from Contributing.mdCONTRIBUTING.md aligns with GitHub conventions. However, you may want to verify that all external links pointing to Contributing.md (with lowercase 'c') are updated. The PR already fixed .github/copilot-instructions.md, but check:

  • Any external documentation
  • GitHub wiki links
  • Issue templates

3. Missing Newline at End of File

Location: .agent_instructions/documentation.md:99

The file ends without a newline character. This is a minor style issue but adding one maintains consistency with Unix conventions.


📋 Specific Recommendations

README.md

  • Consider adding a "Contributing" section near the end that links to CONTRIBUTING.md
  • The "Prerequisites" section could mention specific .NET SDK versions more explicitly (e.g., "Download .NET 8+ from https://dotnet.microsoft.com/download")

CONTRIBUTING.md

  • In the Docker Compose section (line 364), consider adding a note about Docker Desktop memory limits for Windows/Mac users running all services
  • The "First Time Contributing?" section could benefit from a link to the project's Code of Conduct

Code Quality

All changes appear to be documentation-only with no code changes, so there are:

  • ✅ No security concerns
  • ✅ No performance implications
  • ✅ No breaking changes
  • ✅ No test coverage needed

🎯 Summary

Overall Assessment: This is a high-quality documentation PR that meaningfully improves the developer experience. The changes are well-thought-out, follow best practices, and make the project more accessible.

Recommendation: Approve with minor suggestions (duplicate paragraph removal)

Action Items:

  1. Remove duplicate TDD paragraph in CONTRIBUTING.md (lines 240-244)
  2. Add newline at end of .agent_instructions/documentation.md
  3. (Optional) Consider the suggestions above for README/CONTRIBUTING enhancements

Great work on improving the project documentation! 🎉

Copy link
Member

@iancooper iancooper left a comment

Choose a reason for hiding this comment

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

Thanks, I think this is a big improvement.

We need to fix the Post vs Publish issue, but it looks good otherwise

[![CodeScene Code Health](https://codescene.io/projects/32198/status-badges/code-health)](https://codescene.io/projects/32198)
![CodeScene System Mastery](https://codescene.io/projects/32198/status-badges/system-mastery)

**Brighter is a Command Dispatcher and Command Processor framework for .NET**. It enables you to build loosely coupled, maintainable applications using the Command pattern and supports both in-process and out-of-process messaging for microservices architectures.
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we should x-reference Darker here, to show the two working together?

var commandProcessor = host.Services.GetRequiredService<IAmACommandProcessor>();

// Publish to external message broker
await commandProcessor.PublishAsync(new GreetingEvent("World"));
Copy link
Member

Choose a reason for hiding this comment

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

PostAsync here, not PublishAsync. We might want a section on events vs. commands and Publish vs Send. But we need PostAsync for the simple message example

- **Validation**: `[Validation]` - Validate requests before handling
- **Custom middleware**: Create your own attributes

### Outbox Pattern for Reliable Messaging
Copy link
Member

Choose a reason for hiding this comment

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

Don't know if we need to mention switch to DepositPost and dependency on transactionprovider here?

Comment on lines +122 to +128
.UseExternalBus(bus =>
{
bus.UseRabbitMQ(new RmqMessagingGatewayConfiguration
{
AmqpUri = new AmqpUriSpecification(new Uri("amqp://guest:guest@localhost:5672"))
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

We have changed the Brighter API in V10

    .AddProducers(producer =>
    {
        ....
    });

Comment on lines +151 to +159
.Subscriptions(s =>
s.Add<GreetingEvent>(
new Subscription<GreetingEvent>(
new SubscriptionName("greeting-subscription"),
new ChannelName("greeting.event"),
new RoutingKey("greeting.event")
)
)
);
Copy link
Contributor

@lillo42 lillo42 Feb 2, 2026

Choose a reason for hiding this comment

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

We have changed the Brighter API in V10

    .AddConsumers(s =>
       .....
    );

// Configure policies
builder.Services.AddBrighter()
.AutoFromAssemblies()
.Policies(p => p.Add("MyRetryPolicy", Policy.Handle<Exception>().Retry(3)));
Copy link
Contributor

Choose a reason for hiding this comment

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

We have changed the Brighter API in V10, I don't remember how to use it

We also should use the Resiliance Pipeline

Comment on lines +197 to +199
.AutoFromAssemblies()
.UseExternalBus(/* configure transport */)
.UseOutbox(new PostgreSqlOutbox(/* configuration */));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the old Brighter APi


### Quick Setup
1. Fork and clone the repository
2. Ensure you have .NET 8 SDK or later installed: `dotnet --version`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
2. Ensure you have .NET 8 SDK or later installed: `dotnet --version`
2. Ensure you have .NET 10 SDK or later installed: `dotnet --version`

### Quick Setup
1. Fork and clone the repository
2. Ensure you have .NET 8 SDK or later installed: `dotnet --version`
3. Build the solution: `dotnet build Brighter.sln`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
3. Build the solution: `dotnet build Brighter.sln`
3. Build the solution: `dotnet build Brighter.slnx`

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.

4 participants