search

matthewrenze / clean-architecture-demo

A sample app for my online course "Clean Architecture: Patterns, Practices, and Principles" in .NET Framework 4.8
BSD 3-Clause "New" or "Revised" License
1.03k stars 483 forks source link

Update IoC.cs #9

Closed DoomerDGR8 closed 4 years ago

DoomerDGR8 commented 4 years ago

Removed the now obsolete ObjectFactory with recommended alternative

DoomerDGR8 commented 4 years ago

This change will still compile with the latest master and will remove the two warning messages.

matthewrenze commented 4 years ago

@DoomerDGR8,

I've been wrestling with how best to handle your pull request for a few days now. Unfortunately, it's not a simple decision or I would have merged your pull request right away.

Here's the main issue:

The code in this repository is used primarily for my Pluralsight course on Clean Architecture. So, the code needs to stay in sync with the course materials. Otherwise, it will be confusing or potentially misleading to the audience of this course.

On the plus side, I would like to keep this project up-to-date with the latest best practices. So, I think it would be a good idea to merge your pull request into master for everyone.

However, on the down side, if I merge your pull request and other potential patches or suggestions, the code in the repo will no longer reflect the code they're seeing in the online course videos.

I spoke with a few of my peers in online education and they all agree that it's more important to keep the repo code in sync with the course than to modify it for a non-breaking change.

So, I think we have one of two options:

  1. Leave the code as-is for now and if/when I have to update the course (which may happen eventually) I will get everything back up-to-date with the latest versions and best practices.

  2. Create a new branch where we can merge any pull requests to fix deprecated functionality and keep the demo code up-to-date. However, users would need to know to switch from master to this branch to get the patched code.

I'd love to get your input on this before I make a final decision, so feel free to let me know your thoughts.

Thanks!

DoomerDGR8 commented 4 years ago

Hi!

Thank you for reaching out to me!

I'd prefer the course remains as it was recorded. Go for option 1.

Regards, Hassan Gulzar

On Fri, Feb 21, 2020 at 11:34 PM Matthew Renze notifications@github.com wrote:

@DoomerDGR8 https://github.com/DoomerDGR8,

I've been wrestling with how best to handle your pull request for a few days now. Unfortunately, it's not a simple decision or I would have merged your pull request right away.

Here's the main issue:

The code in this repository is used primarily for my Pluralsight course on Clean Architecture. So, the code needs to stay in sync with the course materials. Otherwise, it will be confusing or potentially misleading to the audience of this course.

On the plus side, I would like to keep this project up-to-date with the latest best practices. So, I think it would be a good idea to merge your pull request into master for everyone.

However, on the down side, if I merge your pull request and other potential patches or suggestions, the code in the repo will no longer reflect the code they're seeing in the online course videos.

I spoke with a few of my peers in online education and they all agree that it's more important to keep the repo code in sync with the course than to modify it for a non-breaking change.

So, I think we have one of two options:

1.

Leave the code as-is for now and if/when I have to update the course (which may happen eventually) I will get everything back up-to-date with the latest versions and best practices. 2.

Create a new branch where we can merge any pull requests to fix deprecated functionality and keep the demo code up-to-date. However, users would need to know to switch from master to this branch to get the patched code.

I'd love to get your input on this before I make a final decision, so feel free to let me know your thoughts.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/matthewrenze/clean-architecture-demo/pull/9?email_source=notifications&email_token=AAGJN5VOYFQTA6M2HEOAPPDREAUFBA5CNFSM4KLJODJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMT2WUY#issuecomment-589802323, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGJN5UDWAJXYN5POBCIH6TREAUFBANCNFSM4KLJODJQ .

matthewrenze commented 4 years ago

@DoomerDGR8

Thanks for your understanding and your input.

I'll close out the pull request but keep the proposed change in mind for any future updates to the course.

Thanks again!

Matthew