OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.45k stars 2.4k forks source link

Undo binary breaking change #16996

Closed MikeAlhayek closed 1 week ago

MikeAlhayek commented 1 week ago

Fix #16997

hishamco commented 1 week ago

Where are the binary breaking changes? They are extension methods in the same assembly

MikeAlhayek commented 1 week ago

@hishamco check out the updated description.

hishamco commented 1 week ago

I'm not sure if it's considered as binary breaking-changes while it's an extension methods, so the class name doesn't matter

hishamco commented 1 week ago

I'm not sure who this is related to or affects OC. Did you read about binding redirect it might help in your case

MikeAlhayek commented 1 week ago

I'm not sure if it's considered as binary breaking-changes while it's an extension methods, so the class name doesn't matter

We'll it broke me :)

Let say you have Project A on nuget package that use 2.0 assemblies. Also you have project B that uses 2.1-previews and Project A from nuget.

This will give you an exception similar to:

An unhandled exception was thrown by the application. System.TypeLoadException: Could not load type 'OrchardCore.DisplayManagement.Handlers.SiteServiceCollectionExtensions' from assembly 'OrchardCore.DisplayManagement, Version=2.1.0.0, Culture=neutral, PublicKeyToken=null'.

If Project A was complied with 2.1 previews, then this wont be a problem.

gvkries commented 1 week ago

@hishamco Renaming classes is always breaking, in case of extension methods as well if you invoke them directly.

sebastienros commented 1 week ago

I vote for only shipping major versions of Orchard!

hishamco commented 1 week ago

Renaming classes is always breaking, in case of extension methods as well if you invoke them directly.

Agree, but I don't think there's someone invoke them directly otherwise this is not make sense in case of extension methods :)

Again it's a breaking change