make this more readable.#117
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Aspire AppHost to centralize project registration and renames the database connection string and resource name from "SyncnetPlatform" to "postgres" across the frontend and silo extensions. Feedback indicates that hardcoding the connection string name "postgres" in a generic method limits its reusability and suggests using a parameter instead. Furthermore, the reviewer noted that renaming the database resource to "postgres" is a regression in clarity and could lead to naming collisions, recommending more descriptive resource names.
| builder.Services.AddDbContextPool<DbContextType>(opt => | ||
| { | ||
| opt.UseNpgsql(builder.Configuration.GetConnectionString("SyncnetPlatform"), optionBuilder => | ||
| opt.UseNpgsql(builder.Configuration.GetConnectionString("postgres"), optionBuilder => |
There was a problem hiding this comment.
Hardcoding the connection string name "postgres" inside the generic method RegisterDbContext<DbContextType> limits its reusability. If a different DbContext type is registered using this method, it will still attempt to use the same connection string. Consider making the connection string name a parameter or using a convention based on the type name to maintain the generic nature of the method.
| .WithReference(redis) | ||
| .WithReference(rdbms) | ||
| ; | ||
| .AddDatabase("postgres", databaseName: "SyncnetPlatform"); |
There was a problem hiding this comment.
Renaming the database resource from "SyncnetPlatform" to "postgres" is a regression in clarity. Resource names in Aspire should describe the purpose of the resource (e.g., "SyncnetDb") rather than the underlying technology. Using a generic name like "postgres" can lead to naming collisions if the application expands to use multiple PostgreSQL databases in the future. Additionally, naming the server "npgsql" and the database "postgres" is counter-intuitive.
1 participant
No description provided.