Skip to content

Conversation

martincostello
Copy link
Member

Cherry-pick changes from #137 to make that PR more focussed on fixing OATS.

  • Update NuGet packages to latest versions.
  • Add Grafana logging.
  • Remove redundant configuration file.
  • Add HttpClientFactory.
  • Use primary constructors.
  • Use async methods where available.
  • Use simple using statements.
  • Use structured logging.
  • Use collection expressions.
  • Remove unused fields.
  • Ignore build from docker compose.
  • Add Visual Studio Code configuration file for recommended extensions.

- Ignore `build` from docker compose.
- Remove redundant lines.
Add Visual Studio Code configuration file for recommended extensions.
- Update NuGet packages to latest versions.
- Add Grafana logging.
- Remove redundant configuration file.
- Add HttpClientFactory.
- Use primary constructors.
- Use async methods where available.
- Use simple using statements.
- Use structured logging.
- Use collection expressions.
- Remove unused fields.
@martincostello martincostello added the enhancement New feature or request label Jun 2, 2025
@martincostello martincostello mentioned this pull request Jun 2, 2025
3 tasks
@martincostello martincostello marked this pull request as ready for review June 2, 2025 19:52
@Copilot Copilot AI review requested due to automatic review settings June 2, 2025 19:52
@martincostello martincostello requested a review from a team as a code owner June 2, 2025 19:52
@martincostello martincostello enabled auto-merge (squash) June 2, 2025 19:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the example ASP.NET Core app to use updated dependencies, modern C# patterns, and enhanced observability.

  • Bumped NuGet package versions and removed unused config
  • Added Grafana OpenTelemetry logging/metrics/tracing and HttpClientFactory
  • Migrated controllers to primary constructors, async/await using, structured logging, and collection expressions

Reviewed Changes

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

Show a summary per file
File Description
examples/net8.0/aspnetcore/aspnetcore.csproj Updated package references to latest versions
examples/net8.0/aspnetcore/appsettings.json Added "System": "Warning" to log level settings
examples/net8.0/aspnetcore/appsettings.Development.json Removed redundant development config
examples/net8.0/aspnetcore/Program.cs Configured Grafana via OpenTelemetry and registered HttpClientFactory
examples/net8.0/aspnetcore/Controllers/WeatherForecastController.cs Switched to C# 12 collection expressions and removed unused logger
examples/net8.0/aspnetcore/Controllers/RedisController.cs Refactored to primary constructor DI, structured logging
examples/net8.0/aspnetcore/Controllers/MsSqlController.cs Migrated to async/await with await using and DI for SqlConnection
examples/net8.0/aspnetcore/Controllers/HttpClientController.cs Injected HttpClient, simplified calls and disposal
.vscode/extensions.json Added recommended VS Code extensions
Comments suppressed due to low confidence (4)

examples/net8.0/aspnetcore/Controllers/RedisController.cs:19

  • The LeftPush method no longer accepts a request body but still checks ModelState and pushes a hard-coded value. Reintroduce a [FromBody] parameter or remove the ModelState check to align behavior with its signature.
public async Task<ActionResult<string>> LeftPush()

examples/net8.0/aspnetcore/Controllers/HttpClientController.cs:15

  • The action is declared to return IEnumerable<string> but returns a single string from GetStringAsync. Change the return type to ActionResult<string> or return a collection.
public async Task<ActionResult<IEnumerable<string>>> Get()

examples/net8.0/aspnetcore/Program.cs:27

  • [nitpick] Embedding the database connection string in code makes rotation and environment configuration harder. Consider retrieving it from configuration or a secret store.
var connectionString = "Server=mssql,1433;Database=master;User=sa;Password=Password12345%%;Encrypt=False;TrustServerCertificate=True";

examples/net8.0/aspnetcore/Controllers/HttpClientController.cs:15

  • [nitpick] The new Get endpoint logic (using injected HttpClient) isn’t covered by any unit or integration tests. Add tests to verify response handling and error paths.
public async Task<ActionResult<IEnumerable<string>>> Get()

@martincostello martincostello merged commit 10ad08e into main Jun 4, 2025
10 checks passed
@martincostello martincostello deleted the examples-refactor branch June 4, 2025 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants