search

sonata-project / SonataCoreBundle

[deprecated] SonataCoreBundle
MIT License
319 stars 139 forks source link

Remove DatagridBundle dependency #735

Closed VincentLanglet closed 4 years ago

VincentLanglet commented 4 years ago

Subject

I am targeting this branch, because BC.

The coreBundle does not use a lot the datagrid bundle but it's require in the composer.json.

I would like to require the datagridBundle 3.x in the adminBundle, in order to deprecate the Datagrid directory in favor of classes in the datagridBundle. But the adminBundle actually require the coreBundle which require the datagridBundle 2.x.

Changelog

### Remove
- Remove SonataDatagridBundle dependency
core23 commented 4 years ago

I thought this bundle is deprecated. Can't we replace the datagrid with the 5.x release of the admin bundle? We should finish the following major release instead of deprecating too much.

VincentLanglet commented 4 years ago

I thought this bundle is deprecated. Can't we replace the datagrid with the 5.x release of the admin bundle? We should finish the following major release instead of deprecating too much.

The interface in the SonataAdmin and the SonataDatagrid bundles start to be different because every PR are only made on one of them. For example you added generics to the Pager in the datagridBundle but didn't in the adminBundle. The more we wait, the harder it will be.

For example, if this PR https://github.com/sonata-project/SonataAdminBundle/pull/6029 would have been merged, the Datagrid would have need the FieldDescriptionInterface which is actually in the adminBundle. So it would be impossible to require the datagridBundle in the adminBundle because of circular dependency.

I would first to make common classes in datagridBundle and adminBundle consistent before it'll be too hard.

core23 commented 4 years ago

The more we wait, the harder it will be.

That's exactly the reason why we still have the admin 3.x release, because we wait too long and want to include too many cool features / cleanup.

Back to topic: I'm not sure if we ever use the datagrid bundle inside this repo. Can't we just remove the dependency?

greg0ire commented 4 years ago

I'm not sure if we ever use the datagrid bundle inside this repo.

Apparently we did: https://github.com/sonata-project/SonataCoreBundle/pull/382

VincentLanglet commented 4 years ago

I'm not sure if we ever use the datagrid bundle inside this repo.

Apparently we did: #382

Since the implementation was removed and deprecated, the dependency can be remove now then. Is it ok for you @greg0ire ? :)

core23 commented 4 years ago

This commit removes the datagrid: https://github.com/sonata-project/SonataCoreBundle/commit/c2941d1201fb43777d97709712be9e73f323f424

greg0ire commented 4 years ago

Thanks @VincentLanglet !

VincentLanglet commented 4 years ago

Thanks ! Can you make a release @greg0ire ? :)