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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo binary breaking change #16996

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Undo binary breaking change #16996

merged 3 commits into from
Nov 12, 2024

Conversation

MikeAlhayek
Copy link
Member

@MikeAlhayek MikeAlhayek commented Nov 12, 2024

Fix #16997

@hishamco
Copy link
Member

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

@MikeAlhayek
Copy link
Member Author

@hishamco check out the updated description.

@hishamco
Copy link
Member

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

{
public static IServiceCollection AddSiteDisplayDriver<TDriver>(this IServiceCollection services)
where TDriver : class, IDisplayDriver<ISite>
=> services.AddDisplayDriver<ISite, TDriver>();
=> services.AddDisplayDriver<ISite, TDriver>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such formatting I told you about previously, and you told me it's done by VS :)

Copy link
Member Author

@MikeAlhayek MikeAlhayek Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have better things to do that picking on spaces or discussing. sorry

@hishamco
Copy link
Member

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
Copy link
Member Author

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
Copy link
Contributor

gvkries commented Nov 12, 2024

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

@sebastienros
Copy link
Member

I vote for only shipping major versions of Orchard!

@MikeAlhayek MikeAlhayek merged commit d76f60a into main Nov 12, 2024
17 checks passed
@MikeAlhayek MikeAlhayek deleted the ma/avoid-breaking-change branch November 12, 2024 17:49
@hishamco
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Renaming files is a breaking change
4 participants