-
Notifications
You must be signed in to change notification settings - Fork 2
.Net6 Upgrade #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
yaschete
commented
Feb 28, 2023
- .Net6 Upgrade
- Updated program.cs as a .net6 new coding style and removed startup.cs.
- Code refactor
- Removed unnecessary codes and infrastructure for Carbon.Sample app.
- Created new todo development points for Carbon library.
| namespace Carbon.Sample.API.Application.Controllers | ||
| { | ||
| [ApiController] | ||
| [Route("api/v1/[controller]")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Route kısmı basede olması sıkıntı olabilir; v1,v2 gibi versiyon bazlı controller ve ya routları yönetmeye başlamamız lazım. Base de olursa hep v1 olarak gidebilir. Bu nedenle route u controller a alıp ilgili kullanımı ile alakalı da ufak bir summary yazarsak süper olur
| { | ||
| if (context.InstanceToValidate == null) | ||
| { | ||
| result.Errors.Add(new ValidationFailure() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Buraya bir TODO ekleyelim mi? Error handlig konusun ilerleyişine göre yeniden değerlendirelim burayı
| } | ||
| public interface ISampleRepository : IRepository<SampleEntity>, ISolutionFilteredRepository, IOwnershipFilteredRepository | ||
| { | ||
| Task<SampleEntity> GetByIdAsync(Expression<Func<SampleEntity, bool>> filters); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metot ismi ve parametre uyumuna tekrar bakalım, parametre olarak id alsak daha mantıklı metot ismine göre
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bir TODO ekleyelim mi buraya da? Error handlig konusundan sonra tekrar bakalım buraya
| services.AddScoped<ISampleService, SampleService>(); | ||
| } | ||
|
|
||
| public static void AddCustomCors(this IServiceCollection serviceCollection, string cors) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Buna gerek yok, Carbon configden okuyup ekleyebiliyor cors u, https://github.com/kocdigital/Carbon/blob/9f6673c208f3d2bb45d32e1975cf1ca4d826e9a1/Carbon.WebApplication/CommonStartup.cs#L118
| application.UseHsts(); | ||
| application.UseHttpsRedirection(); | ||
| application.UseCustomRequestLocalization(); | ||
| app.CreateAuthentication("RAM"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Burada RAM kısmı için bir açıklama ekleyelim; neden buraya RAM yazdı vb bilmesi için
| "environmentVariables": { | ||
| "CONFIGURATION_TYPE": "FILE", | ||
| "FILE_CONFIG_PATHS": "appsettings.Development.json", | ||
| "FILE_CONFIG_PATHS": "appsettings.kube_test.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proje içerisinde bir app setting bırakalım; bu appsettingsde şifre, url vb olmamalı. Ortamlar, uygulama nasıl debug edilir neler gereklidir bunun için ayrı bir eğitim içeriği olmalı.
| "FILE_CONFIG_PATHS": "appsettings.kube_test.json", | ||
| "ENVIRONMENT_TYPE": "Kestrel", | ||
| "ASPNETCORE_ENVIRONMENT": "Development" | ||
| "ASPNETCORE_ENVIRONMENT": "kube_test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Development kalması daha sağlıklı
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error handling konusundan sonra bu resxler ne olacak vb buraya geri dönmeyi unutmayalım
| <?xml version="1.0" encoding="utf-8"?> | ||
| <configuration> | ||
| <packageSources> | ||
| <clear /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Buraya bir yorum satırı ekleyelim; eğer nuget.org dışında bir paket sağlayıcı varsa buraya eklenmeli, eğer yok ise bu dosyaya ihtiyaç olmayabileceği tarzında.