umbraco / UmbracoDocs

The official Umbraco Documentation
https://docs.umbraco.com
MIT License
266 stars 773 forks source link

Delete code examples that update Program.cs or Startup.cs #5709

Closed erikjanwestendorp closed 1 week ago

erikjanwestendorp commented 9 months ago

What type of issue is it? (Choose one - delete the others)

Discussion/Wrong documentation

What article/section is this about?

Several articles like: https://docs.umbraco.com/umbraco-cms/extending/section-trees/sections https://docs.umbraco.com/umbraco-cms/extending/content-apps etc.

Describe the issue

In a discussion about extension methods, the advice is given to make no changes to the Startup.cs or Program.cs files. I believe it is a good idea to remove the code examples from the docs that make changes to the Startup.cs or Program.cs. Instead, examples using an IComposer can be added.

alina-tincas commented 9 months ago

Hi @erikjanwestendorp thank you for opening this issue! We are currently aware of it and have this is a task in our backlog πŸ™

We will look into it in the next sprint from January πŸ‘

alina-tincas commented 9 months ago

@erikjanwestendorp examples with startup and program.cs below v13 are fine.

It is only from v13 that the examples with startup.cs are not valid anymore as it is only from v13 that there is no startup.cs anymore.

Some examples from v13 have also been updated to use program.cs in this PR instead of startup.cs: https://github.com/umbraco/UmbracoDocs/pull/5669

As for if we should remove from v13 the examples with startup we will need to discuss this with our team so I will get back to you on this πŸ™

Meanwhile, I have noticed that some of your PRs you are removing the examples from below v13 and also removing the program.cs examples from v13 which will not be approved in this case unless there is a change related to startup.cs examples on v13.

erikjanwestendorp commented 9 months ago

@alina-tincas Thanks for your reply! πŸ˜„ As far as I know the advice in general is: Do not touch the 'Program.cs/Startup.cs' files and use an ICompser instead. My PRs aren't so much about changing Startup.cs to Program.cs or updating to the minimal hosting model. My PRs removes code samples that update Startup.cs or Program.cs in general, since that is the advice (These articles also provide examples with an IComposer already). I think it is good to remove these examples, otherwise the documentation goes against this advice πŸ˜„.

alina-tincas commented 9 months ago

I understand! We will discuss this with our CMS developers on what is the best approach in this case, if those examples should indeed be removed. I will get back to you on this πŸ‘

jonat123 commented 8 months ago

Hey Erik and happy new year πŸŽ‰

I want to let you know, that I have talked with the Lead developer for the CMS team, and he is fine with removing the examples that updates make changes to the startup.cs and program.cs files and replacing them with examples using composers instead(If they don't already have composer examples) πŸ˜„

There is only one thing to be aware of and that is examples that are using middleware. For Middleware it can also be done in a Composer, using the pipeline filter 😊

erikjanwestendorp commented 8 months ago

@jonat123 Happy new year! Thanks for the update πŸ˜„! Good to hear that the examples can indeed be removed/replaced

sofietoft commented 3 months ago

Hi @erikjanwestendorp ! πŸ‘‹ Long time to see! Hope you're doing well, and congratz on getting your MVP status renewed last week 😎

I've been looking at this one, and I like the idea of keeping the docs streamlined in terms of what we recommend - eg. do we recommend using composers versus making changes to the Program.cs file?

Were you working on this based on a list of articles referencing the Program.cs file? I'll be making one, but don't want to, if one already exists! 😁

erikjanwestendorp commented 3 months ago

Mucho gracias @sofietoft! πŸ˜„ Indeed, it's been a long time! Are you back at the Umbraco office after your leave (I assume you were on leave)? Sorry for my late response, I was on leave myself last month (πŸ‘ΆπŸΌ ).

To be honest, I haven't made a list yet and I started going through the articles, but it is indeed a good idea to make a list to have a clear overview.

sofietoft commented 3 months ago

I have indeed been on leave for the past 10 months, yes (πŸ‘ΆπŸ»πŸ‘ΆπŸ»πŸΌ) , but it's very good to be back at work again! Kids are great and all, but I'm really not the "stay at home mom" type 😁 Congratz on the little one! Hope you had a great leave!

Fair enough about the list. I just wanted to make sure I didn't start making a list, if one had already been made. Work smart, not hard, right? πŸ˜‰

I'll get to work on a list of articles where code samples make changes to the Program.cs file πŸ’ͺ

sofietoft commented 3 months ago

Hi @erikjanwestendorp πŸ‘‹

I've just made a PR related to this issue: #6243

With this, I've broken this issue down a bit, to only revolve around using the Program.cs for registering dependencies. We'll take a look at the rest of the scenarios after that fact, if necessary!

After talking with our CMS PO, Lasse, about this, we agreed that we want to try to keep opinions and personal preference out of our documentation - if it all possible! 🀞

That's why I've now rewritten the IoC article a bit, so that it presents a couple of strategies while leaving it up to the individual user to chose the one best fitting their scenario.

You're welcome to take a look and see if it makes sense. The next step is to reference this article in other articles where dependencies are registered, and make sure that we're not too opinionated in those articles either πŸ˜…

I hope it all makes sense! Let me know if you have comments, thoughts, questions...

sofietoft commented 1 week ago

Hi @erikjanwestendorp ! πŸ‘‹

I think we should consider this Issue resolved. Looking at the initial messages, it seems that you've been wanting to remove the examples that register dependencies directly in the Startup.cs/Program.cs files - and this is to some degree what my two related PRs do.

If you still feel we should look into the other places where changes are made to the Program.cs (or Startup.cs) files, you're most than welcome to open a new issue, and we'll take a closer look into it πŸ’ͺ

As always, THANK YOU for helping us keep the docs up to speed! πŸ™Œ #H5YR