Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

EPIC: eShop Modernization/Refresh #2070

Open
12 of 16 tasks
adityamandaleeka opened this issue Mar 22, 2023 · 13 comments
Open
12 of 16 tasks

EPIC: eShop Modernization/Refresh #2070

adityamandaleeka opened this issue Mar 22, 2023 · 13 comments

Comments

@adityamandaleeka
Copy link
Contributor

adityamandaleeka commented Mar 22, 2023

Creating a top-level issue for us to track the ongoing work to modernize/refresh eShop.

@adityamandaleeka
Copy link
Contributor Author

cc: @erjain @ReubenBond

@ReubenBond
Copy link
Collaborator

ReubenBond commented Mar 22, 2023

EDIT: Merged into top comment * [ ] Migrate from ad-hoc `IConfiguration` usage to strongly typed Options * [ ] Replace Autofac with simple Microsoft.Extensions.DependencyInjection usage

@ardalis
Copy link
Collaborator

ardalis commented Mar 22, 2023

Why would you replace Autofac with something that has fewer features?

Also regarding IOptions be sure to read this first: https://www.dabrowski.space/posts/asp.net-options-why-you-should-not-use-it/

@ReubenBond
Copy link
Collaborator

ReubenBond commented Mar 22, 2023

EDIT: Merged into top comment * [ ] Use `ILogger` instead of injecting `ILoggerFactory`. Example: https://github.com/dotnet-architecture/eShopOnContainers/blob/0740fd42b12cbcf2e7ee0b69585bf01312adeddd/src/Services/Ordering/Ordering.API/Application/DomainEventHandlers/OrderStockConfirmed/OrderStatusChangedToStockConfirmedDomainEventHandler.cs#L14

@ReubenBond
Copy link
Collaborator

ReubenBond commented Mar 22, 2023

Why would you replace Autofac with something that has fewer features?

Autofac is undeniably more powerful and feature-rich. For the purposes of this project, however, I would rather we be explicit about what types in the application are services for DI and what interfaces they satisfy. It certainly should not be taken as endorsement of MEDI over Autofac.

Also regarding IOptions be sure to read this first: https://www.dabrowski.space/posts/asp.net-options-why-you-should-not-use-it/

The issue today is that stringly IConfiguration access is sprinkled around the codebase. The IOptions<T> type isn't supposed to be an aspiring GoF/OOP pattern, it's just a practical mechanism which works with C#'s type system and DI to attach certain functionality to a category of types. Specifically, configuration types and the functionality includes binding, programmatic configuration, validation, and post-config. In this case, the intention is to guide developers into a pattern which we consider to be a good pattern.

@ReubenBond
Copy link
Collaborator

ReubenBond commented Apr 12, 2023

EDIT: Merged into top comment * [ ] Fix DataProtection issues by configuring a data protection provider

@ReubenBond
Copy link
Collaborator

Many of these are addressed in #2107

@LockTar
Copy link

LockTar commented Jun 12, 2023

Is see that src/Services/Ordering/Ordering.API/Infrastructure/Filters/HttpGlobalExceptionFilter.cs (and from other services) is deleted. What is the reason for this? How are validation errors from in example FluentValidation handled. But also xxxDomainExceptions?

@davidfowl
Copy link
Collaborator

We’re going to bring back some of it and use problem details instead of the custom format.

@markheath
Copy link

Really great to see the plans to modernize eShopOnContainers - I have used it for a number of demos and developer training over the years.

One thing I noticed is that it seems like the latest version is no longer logging to Seq (presumably because of the move away from Serilog). Will this be restored or are there plans to send the logs elsewhere?

@pjmlp
Copy link

pjmlp commented Jul 3, 2023

I suggest to also update the Wiki, as the architecture documentation is quite out of the date and seems to relate mostly to the .NET 5 rewrite.

To pick on the previous comment as an example, Serilog alongside Seq is still documented as the current logging approach.

@ashutosh-tutwani
Copy link

Are we also implementing OpenTelemetry with this ongoing task to modernize the eshop app?
If not, then can we add it?

cc @davidfowl @ReubenBond

@eerhardt
Copy link
Contributor

@adityamandaleeka - can this issue be closed now that we have https://github.com/dotnet/eShop?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants