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

Consider removing the EqualsOrdinalIgnoreCase extension method (OSOE-915) #296

Open
sarahelsaig opened this issue Nov 12, 2024 · 5 comments

Comments

@sarahelsaig
Copy link
Member

sarahelsaig commented Nov 12, 2024

In get this error in OrchardCore 2.0 too often:

TestApiController.cs(105, 62): [CS0121] The call is ambiguous between the following methods or properties: 'System.StringExtensions.EqualsOrdinalIgnoreCase(string, string?)' and 'OrchardCore.Modules.StringExtensions.EqualsOrdinalIgnoreCase(string?, string?)'

This is because the extension method in OC here.

Jira issue

@github-actions github-actions bot changed the title Consider removing the EqualsOrdinalIgnoreCase extension method Consider removing the EqualsOrdinalIgnoreCase extension method (OSOE-915) Nov 12, 2024
@Piedone
Copy link
Member

Piedone commented Nov 12, 2024

Our extensions are in the System namespace, so accessible without further ado if you use string. The OC ones are in OrchardCore.Modules, so less easily discoverable.

So, I'd prefer have ours, or change the namespace in OC.

Can you link a few samples where you had this clash?

@sarahelsaig
Copy link
Member Author

I don't commit build errors into git so I can't link samples. I think the most common example is if you are in a service that injects IClock, ILocalClock or IApplicationContext then you can't use our extension method because those interfaces live in the same namespace as OC's extension class. Also if you want to use EqualsOrdinalIgnoreCase and ContainsOrdinalIgnoreCase in a service that injects any of the above services, you can't. (technically you can, if you always use the fully qualified namespace for the services instead of using OrchardCore.Modules, but then you'll have to fight with the IDE forever)

@Piedone
Copy link
Member

Piedone commented Nov 12, 2024

I think the solution here is to change the namespace in OC.

@sarahelsaig
Copy link
Member Author

Try to push through an upstream breaking change? 😱 I'll let you fight that windmill if you want.

@Piedone
Copy link
Member

Piedone commented Nov 12, 2024

It is breaking, but I doubt many would notice :). Nevertheless should be v3. In any case, I don't think we should change HL.

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

No branches or pull requests

2 participants