search

silverstripe / silverstripe-asset-admin

Silverstripe assets gallery for asset management
BSD 3-Clause "New" or "Revised" License
19 stars 78 forks source link

Versioned module required #1340

Closed alessandromarotta closed 1 year ago

alessandromarotta commented 1 year ago

Module broken without Silverstripe/Versioned installed but there's not dependency in composer.json

@see https://github.com/silverstripe/silverstripe-versioned/pull/399

michalkleiner commented 1 year ago

I wonder whether adding the extension in this module is the right place as well since it's being added to the File class from the assets module. The assets module can function without versioning even though it's rarely used that way. It's not a hard dependency there. The assets admin (this) module should also work without versioning, even though it's probably not fully decoupled in all places.

GuySartorelli commented 1 year ago

This seems very much like it belongs in versioned, where it already is. Framework requires assets. Assets doesn't (theoretically) need versioned. And this admin module as Michal points out is definitely the wrong place for this config, which is about how assets and versioned work together.

I'm going to close this and https://github.com/silverstripe/silverstripe-versioned/pull/399 - but if you have a strong argument for why this change should be made please do comment and we'll see if we need to reconsider.

alessandromarotta commented 1 year ago

Assets doesn't require versioning but the Versioned module (installed by the Admin module) force you to have it. Actually which need the Versioned module is the Assets Admin because has code coupled with Versioned module.