Remove 'Storage' from OC.Media.Azure feature Id

[hishamco]

[jtkech]

Yes you are right, I asked to do it like this for the Media.AmazonS3 module, if you look at the comments of the related merged PR.

But we didn't update Media.Azure.Storage to Media.Azure as it would be a breaking change ;)

[hishamco]

We could mentioned this at breaking changes section in the module's README

[Skrypt]

Provide a migration path. A SQL Query or something for people to know what to do. Maybe just enabling back that feature but it will also add a new feature in the database while the older feature will still be there. It's not a clean migration path.

[hishamco]

While this is a feature id, I'm not sure what's the suitable way for migration. @jtkech any suggestion?

[Piedone]

I don't really see much point in changing the ID anyway, but I'm adding OrchardCore.Media.Azure.ImageSharpImageCache here: https://github.com/OrchardCMS/OrchardCore/pull/15028. So, the distinction of the original ID will be necessary and I think this can be closed.

[hishamco]

It's fine for OrchardCore.Media.Azure.ImageSharpImageCache while it's Azure related feature, but in terms of storage we should have Azure & Amazon

Either we accept this if all agree or do it before 2.0.0

[Piedone]

But what's the reason to change the ID here, what benefit does it bring? I only see a breaking change here without a clear goal, and after https://github.com/OrchardCMS/OrchardCore/pull/15028 is merged, a confusing feature ID.

[hishamco]

But what's the reason to change the ID here, what benefit does it bring?

Consistency

For OrchardCore.Media.Azure.ImageSharpImageCache I prefer OrchardCore.Media.Azure.ImageCache

No rush we can defer this to 2.0.0 while it's breaking change

[Piedone]

What would it be consistent with, exactly? If it were a single feature in the module then I get it but then again it won't be. And having OrchardCore.Media.Azure and OrchardCore.Media.Azure.ImageCache would imply the latter depending on or being part of the former. This will be the case once my PR is merged, but wouldn't be if OrchardCore.Media.Azure will be the storage feature in itself.

[hishamco]

What would it be consistent with, exactly?

With OrchardCore.Media.AmazonS3

[Piedone]

That module came later so maybe it needs to be consistent with the Azure one ;). Then again, what I've written above stands. These changes won't be needed in the foreseeable future.

[hishamco]

As JT said here maybe it's our fault at the beginning

While we accepting the breaking changes in minor release, so it's fine to fix IMHO

[Piedone]

My problem is that it's not making things better.

[github-actions[bot]]

This pull request has merge conflicts. Please resolve those before requesting a review.

[hishamco]

Closing this might revisit before 2.0.0