kgrzybek / modular-monolith-with-ddd

Full Modular Monolith application with Domain-Driven Design approach.
MIT License
10.82k stars 1.7k forks source link

refactor: Use new collection initializers #292

Closed AlmarAubel closed 8 months ago

AlmarAubel commented 8 months ago

I've made some updates in our codebase to leverage the new list initializer syntax introduced in C# 12. This change aims to enhance readability and maintainability of our code.

However, I encountered an issue with StyleCop. Currently, StyleCop has a known problem (#3745, #3687) that triggers SA01010 as a false positive when using this new syntax. Until this is resolved in StyleCop, we might see this warning.

I'd appreciate your feedback on these changes. Please share your thoughts if you like this kind of pull request And if you're okay with the interim StyleCop warning we'll encounter.

bistok commented 8 months ago

I was tempted to do this change when migrated to .NET 8, but not do it because the Stylecop warnings, I see they merged today the commit that fixed this and marked it to the next release, I think we can live some time because they will fix it when releasing the next package version.

kgrzybek commented 8 months ago

@AlmarAubel

I really like this type of pull request.

However, the problem lies with these warnings. I want to treat this project to be as close to the production implementation as possible. In projects I participate in, we treat all warnings as errors, and I consider it a good practice.

Therefore, I would prefer to wait with merging this pull request until a new version of StyleCop is released. WDYT?

AlmarAubel commented 8 months ago

Yes totally agree, at my professional projects we also treat warnings as errors. So let's wait. I will monitor the release of the new version of Stylecop and will add the version bump to this pr once released.

kgrzybek commented 8 months ago

Good, let's wait.

Meanwhile, I removed all warnings and enabled "warnings as errors" in https://github.com/kgrzybek/modular-monolith-with-ddd/commit/fed5f5fd94c7a016cf2186204e4a65e1a253d7e6

AlmarAubel commented 8 months ago

I've resolved the issue using Stylecop and updated the package accordingly. Apologies for the additional commits resulting from an error while merging from master. I hope you can squash merge this commit to streamline the changes.

kgrzybek commented 8 months ago

Good job @AlmarAubel, thanks for the active monitoring of StyleCop repo!