Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC 0006: Proposed project structure for Project UniCore 馃 #6

Open
wants to merge 2 commits into
base: master
from

Conversation

@Shazwazza
Copy link
Member

commented May 20, 2019

Organize your life around your dreams and watch them come true

See RFC document (in this PR): 0002-project-unicore-structure.md

Summary

Title: Project UniCore - Project Structure 馃

This RFC is a child of Project UniCore - Introduction & Strategy
We would like to re-organize the .Net solution/project structure and the resulting Nuget packages for the Umbraco CMS.

@Shazwazza Shazwazza changed the title RFC: Proposed project structure for Project UniCore RFC: Proposed project structure for Project UniCore 馃 May 20, 2019

@Shazwazza Shazwazza changed the title RFC: Proposed project structure for Project UniCore 馃 RFC 0006: Proposed project structure for Project UniCore 馃 May 21, 2019

@anthonydotnet

This comment has been minimized.

Copy link

commented May 25, 2019

Umbraco.Infrastructure implies hosting infrastructure.

Should be something like Umbraco.Implementation
or Umbraco.Concrete

@mattbrailsford

This comment has been minimized.

Copy link

commented May 26, 2019

I get the reasonings for splitting things up, but it doesn鈥檛 half make things harder to follow. Especially if we are talking separate nuget packages too.

I recall recently that a lot of consolidation took place to try and simplify the project structure so it would kind of feel a shame to go down a route that breaking everything back up again (all be it sure, this time it鈥檚 more considered).

Is the breaking up purely to allow things to be converted gradually one at a time? Or is there some other reasons for it? I鈥檇 probably prefer the structure to be kept simpler if at all possible.

If it is split, would there be future plans to pull them all back together?

Could we do something where we keep things more together and have something like 鈥淯mbracoCms.Framework鈥 and then 鈥淯mbracoCms.Standard/Core鈥 (Probably not the names we鈥檇 use, but I鈥檓 going with them for the example) and then move the code across from Framework to the Standard/Core libs as they are converted?

@JimBobSquarePants

This comment has been minimized.

Copy link

commented May 28, 2019

I see a lot of problems with this approach.

I understand the driver. You've just done a major release with breaking API changes and you don't want to do that again too soon. However, you're shooting yourself in the foot following this approach and it's going to come back to haunt you.

A couple of high level notes:

Globally You get none of the performance benefits of the new CLR and limit your API scope by targeting netstandard in your base projects. By limiting API changes you are essentially only changing the target framework. A lot of work which yields almost no benefit.

It's impossible for the reader to go into great detail regarding the structural layout. The diagram gives no clear indication of the dependency graph so it's hard to discern what depends on what. You should make that crystal clear if only to increase your understanding of potential pitfalls. The separation of projects does seem odd to me however.

ImageProcessor is a retired project (I'm retiring it the day V1 ImageSharp is released) and SqlCe end of life is Jan 2021 so you might as well give up on that straight away.

Net Framework is obsolete. 4.8 is the last version. NET Core 3.1 with LTS is coming in November and a year later NET 5 will be released. Based on the slow cadence of current Umbraco releases I can only see you drift further off the technology pace. This is going to hurt uptake.

I have no horse in this race, I don't use Umbraco professionally anymore but as it stands, based on the current status of V8, and these future plans I wouldn't recommend it as a solution either.

@bergmania

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Hi @anthonydotnet, thanks for the feedback. We are open to rename. The Infrastructure name is taken from Clean Architecture. There is more information in this whitepaper from Microsoft: https://dotnet.microsoft.com/download/thank-you/aspnet-ebook

When taking the information in the whitepaper into account, I personally don't see the names Umbraco.Implementation or Umbraco.Concrete as real alternatives, because the application core still should have lots of implementations.

@bergmania

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Hi @mattbrailsford,
The big issue is that, we have a lot of code in the current Umbraco.Core that is not compatible with NetStandard.
We therefore seek for an application core, that is dependency-less. This requires us to split the project into two: Umbraco.Core (dependency-less) and Umbraco.Infrastructure (lots of dependencies). Furthermore we need to split out implementations that do not support NetStandard, into their own projects/assemblies.

Currently there is no plans to pull them all together in the future.

What I personally like with this structure, is the clean cut to dependencies that is interchangeable, e.g. Persistence (SqlCe vs SqlServer).

@bergmania

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Hi @JimBobSquarePants,

I'm not sure I understand why you think the diagram is not indicating the dependencies? There are arrows? So e.g. Umbraco.Core does not know other projects, but is known by everyone.

I think your suggestions about SqlCE and ImageProcessor is out of scope of this RFC, but I see them as good candidates for individual RFCs.

@JimBobSquarePants

This comment has been minimized.

Copy link

commented May 28, 2019

You might want to read the RFC again. The diagram is distinctly unclear. ModelsBuilder, for example, clearly has a dependency on Core and Infrastructure but is not connected.

Both ImageProcessor and SqlCE are identified in the diagram.

@Shazwazza

This comment has been minimized.

Copy link
Member Author

commented Jun 4, 2019

@mattbrailsford - before v8 we had a mess of projects which didn't make sense, we had to go with the approach of moving everything back in to the minimum amount of projects and also since there was limited time to make things nicely abstracted, this was also required. Now that we have removed the old legacy bits, we now have a chance to split things up into where it makes sense. There are plenty of benefits to this and perhaps the RFC should be updated in "Motivation" to further describe this (looks like we forgot to add more stuff there since there's a "TODO" there :) ). Like @bergmania points out, this split is necessary to split up things that are not netstandard compliant or not. The split also makes the transition far easier to do but then also gives us the ability to host only the back office or only the front-end of Umbraco which has been a request for a long time since this makes load balancing far nicer. The main thing about this split is that it's the Phase 1 of this whole thing before we even start looking at .Net Core which means once we get to this stage, Umbraco will work as-is but with this new structure with as much targeting net standard as possible. Then the journey to convert Umbraco.BackOffice and Umbraco.Web to net core will commence.

@JimBobSquarePants Yes this is phase 1 (the parent RFC to this is here #2). Phase one is the split so that as much as possible is running net standard and we want to be able to run umbraco as-is with this split in phase 1. We will not be releasing an umbraco version after phase 1, this is just part of the journey to net core. Phase 2 is the net core migration which will be done in only 2 projects: Umbraco.BackOffice and Umbraco.Web - these are the hosts which will run .Net core. Phase 1 is necessary to get to Phase 2.

Globally You get none of the performance benefits of the new CLR and limit your API scope by targeting netstandard in your base projects. By limiting API changes you are essentially only changing the target framework. A lot of work which yields almost no benefit.

Yes, if the hosts are running in net framework - but in phase 2 they will be running net core.

ImageProcessor is a retired project (I'm retiring it the day V1 ImageSharp is released) and SqlCe end of life is Jan 2021 so you might as well give up on that straight away.

We are aware of this - again this is phase 1. We want to be able to run umbraco as-is without .net core for phase one. Phase 2 is the net core switch, which means that some other imaging implementation can be done. This is the same story for sqlce. Part of Umbraco.Core will be our own imaging abstractions and the Umbraco.Imaging.ImageProcessor will be the netframework implementation of that. In phase 2 someone can create any alternative implementations.

You might want to read the RFC again. The diagram is distinctly unclear. ModelsBuilder, for example, clearly has a dependency on Core and Infrastructure but is not connected.

Sure the MB dependency is an odd one I think might exist on this diagram by mistake. I think it is there because there was an intention to only rely on it's abstractions but then realizing it's actually just a plugin on umbraco so it's not part of any of this umbraco changes anyways. We can update.

The separation of projects does seem odd to me however.

Hopefully with knowing about Phase 1 and 2, does this make more sense? I think the RFC can be updated to be more clear on phase 1 and 2 and the Umbraco.BackOffice and Umbraco.Web (net core in phase 2) to be more clear. Otherwise what else are you proposing and/or does this need more explanation?


We listed the alternatives to not doing this split which is here: https://github.com/umbraco/rfcs/blob/61070dea5f3130cfab573636948efb6fd5bd4884/cms/0000-project-unicore-structure.md#alternatives

@JimBobSquarePants

This comment has been minimized.

Copy link

commented Jun 4, 2019

Thanks @Shazwazza That makes everything a lot more clear 馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can鈥檛 perform that action at this time.