Skip to content

Clarify DI package descriptions#1163

Merged
mtmk merged 1 commit into
mainfrom
clarify-di-descriptions
Jun 12, 2026
Merged

Clarify DI package descriptions#1163
mtmk merged 1 commit into
mainfrom
clarify-di-descriptions

Conversation

@mtmk

@mtmk mtmk commented May 27, 2026

Copy link
Copy Markdown
Member

Update the NuGet for the two DI packages to state what each one is for: NATS.Client.Hosting (minimal deps, AOT) vs NATS.Extensions.Microsoft.DependencyInjection (ad-hoc JSON by default), each pointing at the other.

@synadia-claude-reviewer

synadia-claude-reviewer Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 1m 4s —— View job


PR Review

  • Gather context
  • Read changed files
  • Analyze changes
  • Post review

The two changed lines are NuGet <Description> fields — no runtime behavior is touched.

Accuracy check against the actual code:

NATS.Client.Hosting:

  • "minimal dependencies" ✅ — only Microsoft.Extensions.Hosting.Abstractions + NATS.Client.Core
  • "AOT support" ✅ — no ad-hoc JSON serializer, no reflection-heavy paths; AOT-friendly by design
  • "Registers NatsConnection" ✅ — also registers INatsConnection (and pool variants); the simplification is fine for a package description
  • Cross-reference to NATS.Extensions.Microsoft.DependencyInjection

NATS.Extensions.Microsoft.DependencyInjection:

  • "ad-hoc JSON serialization enabled by default" ✅ — NatsBuilder defaults to NatsClientDefaultSerializerRegistry.Default when no registry is overridden (line 45 of NatsBuilder.cs)
  • "Registers NatsConnection and INatsClient" ✅ — Build() also registers INatsConnection and pool types, but naming the two most distinctive registrations is a reasonable summary
  • Cross-reference to NATS.Client.Hosting

No correctness, security, or performance issues. The descriptions accurately capture the key distinction between the two packages and will help users pick the right one. LGTM.

@scottf scottf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mtmk
mtmk merged commit b11eda2 into main Jun 12, 2026
6 checks passed
@mtmk
mtmk deleted the clarify-di-descriptions branch June 12, 2026 11:43
This was referenced Jun 15, 2026
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