search

sonata-project / dev-kit

Development kit of the Sonata-Project
https://master-7rqtwti-ptm4dx6rjpjko.eu-5.platformsh.site/
42 stars 42 forks source link

Recipes for Symfony Flex #409

Closed covex-nn closed 3 years ago

covex-nn commented 6 years ago

This article contains a plan for creating recipes for sonata-project bundles. Each bundle must be prepared according to my proposal:

  1. It should be possible to create a default configuration, that allows to run cache:clear without errors
  2. It should be possible to create an additional configuration for each supported persistence mechanism, that will make a bundle fully functional after installing some required additional packages

A default configuration will be a bundle's recipe. Additional configuration will be a recipe for a new package. This new package, a pack, will contain composer.json with all requirements for particular persistence mechanism.

A bundle, that will be installed without a pack (and without additional configuration) can throw exceptions with instructions for developer how to set it up with installing a pack.

MediaBundle

SeoBundle and CacheBundle

Maybe a default configuration could be created? Or bundle configuration is enough?

NotificationBundle

PageBundle

ClassificationBundle

TranslationBundle

ArticleBundle

TimelineBundle

CommentBundle

DashboardBundle

UserBundle and NewsBundle

It is not possible to create a recipe for these bundles, because they depends on FOSUserBundle, that does have and cannot have a recipe.

FormatterBundle

egeloen/ckeditor-bundle should be replaced with friendsofsymfony/ckeditor-bundle, but it is a BC break

ClassificationMedia

No plans, a recipe will be possible after next major release of MediaBundle and ClassificationBundle

kunicmarko20 commented 6 years ago

Make a db_driver configuration parameter optional with no_driver default value

I think this will be a bc break 😞

Set admin.enabled configuration parameter == defaultFalse

same

SeoBundle and CacheBundle

Maybe a default configuration could be created? Or bundle configuration is enough?

If there is any configuration that has to be set in docs, then we add it to recipe.

So basically, we can only do: SeoBundle, CacheBundle, PageBundle, ClassificationBundle, ArticleBundle, TimelineBundle , DashboardBundle?

covex-nn commented 6 years ago

I believe, that changes for MediaBundle is not a BC break, i will create a PR soon, so we can discuss it there. I hope MediaBundle will have a recipe soon. And ArticleBundle depends on MediaBundle =)

PageBundle and DashboardBundle depends on NotificationBundle. And we can create a recipe for NotificationBundle, actualy. Changing admin.enabled is optional, and it is BC break, yes. With that change i want to enable admin only when backend is sonata.notification.backend.doctrine and Entity is available. Otherwise it will be disabled by default configuration in current release.

If there is any configuration that has to be set in docs, then we add it to recipe.

:+1:

kunicmarko20 commented 6 years ago

oh, if by default it is empty, then no, I thought it is orm by default. That won't be a bc break for media then.

covex-nn commented 6 years ago

Btw, what package name is more correct: sonata-project/media-odm-pack or sonata-project/media-mongodb-pack?

covex-nn commented 6 years ago

Updated PR for sonata-project/notification-bundle, see https://github.com/symfony/recipes-contrib/pull/302. Recipe contains only config/packages/sonata_notification.yaml file

There is a problem with PageBundle: symfony-cmf/routing-bundle has no stable release with Symfony4 support =(

kunicmarko20 commented 6 years ago

Media pack name depends on:

that depends whether you target ODM or plain MongoDB

Copied from Slack conversation I never used it so I am not sure, I guess ODM?

kunicmarko20 commented 6 years ago

So, for now, we need:

sonata-project/media-orm-pack
sonata-project/media-odm-pack
sonata-project/media-phpcr-pack
sonata-project/notification-orm-pack
sonata-project/classification-orm-pack
sonata-project/classification-odm-pack

this repositories created?

covex-nn commented 6 years ago

Yes, that's right. It will be possible to create a recipes for sonata-project/classification-bundle, i just checked (i guess, that before creating recipes, composer.json must be updated. i will create a PR)

kunicmarko20 commented 6 years ago

ok, will contact @rande about creating repositories

kunicmarko20 commented 6 years ago

cc @greg0ire, @jordisala1991, @core23, @OskarStark, @Soullivaneuh before I contact @rande, do you agree we continue this way and use packs?

core23 commented 6 years ago

do you agree we continue this way and use packs?

Never used symfony flex /w packs. Do we need this many packs or can we create a general sonata/doctrine-pack?

covex-nn commented 6 years ago

@core23 sonata/doctrine-pack is too generic, it cannot bring anything for sonata-project/media-bundle except doctrine/doctrine-bundle and doctrine/doctrine-orm - and symfony/orm-pack already contains such dependencies.

With my proposal, see https://github.com/sonata-project/SonataMediaBundle/issues/1419, after composer require sonata-project/media-orm-pack developer will receive:

  1. sonata-project/media-bundle and sonata-project/doctrine-orm-admin-bundle from pack's dependencies; also doctrine/doctrine-bundle with doctrine/orm will be installed
  2. A default configuration from MediaBundle recipe, see comment
  3. A storage configuration from MediaOrmPack recipe, see the same comment
  4. Three Entity classes, see gist

And after bin/console doctrine:schema:update --force developer will have fully functional MediaBundle

rande commented 6 years ago

Do you want 1 repositories with all packs and split it like symfony ? or do you want many repositories ?

The pack solution looks great!

covex-nn commented 6 years ago

Each repository must contain composer.json file, unique for each pack. So, it cannot be splited, i guess.

Please create many repositories =)

rande commented 6 years ago

@covex-nn it is possible: https://github.com/splitsh/lite

covex-nn commented 6 years ago

@rande i've starred that repo =) but anyway creating one more repository for all recipes does not bring us closer to achieving the goal. All recipes are not related to each other, each repository will contain only composer.json, there won't be even .travis-ci.yml (recipes will be tested in symfony/recipes-contrib, also #388 is about recipe testing). @kunicmarko20 what do you think?

Also, i think that it is too early for sonata-project/classification-orm-pack and sonata-project/classification-odm-pack. ClassificationBundle is not ready for using with MongoDB even with a pack. Right now it supports only Doctrine ORM; it requires sonata-project/doctrine-orm-admin-bundle; all services depends on doctrine; also see https://github.com/sonata-project/SonataClassificationBundle/issues/262. So, i think that recipe for ClassificationBundle should contain Entities, like PageBundle will contain.

kunicmarko20 commented 6 years ago

I also think we should have separate repositories, didn't see any pack going with just 1 generic repo. So then we just need:

sonata-project/media-orm-pack
sonata-project/media-odm-pack
sonata-project/media-phpcr-pack
sonata-project/notification-orm-pack

for now? @covex-nn

covex-nn commented 6 years ago

Yes, just 4 new repositories

covex-nn commented 6 years ago

I prepared gists with Entity classes for NotificationBundle, MediaBundle and PageBundle, please review if you have time. I will use these classes in recipes

kunicmarko20 commented 6 years ago

JMS is an optional dependency, I think it can't be done this way?

covex-nn commented 6 years ago

JSM serializer bundle is required dependency only for MediaBundle, and i added * @Serializer\Groups only for media entities. For notification and page entities i added * // Serializer\Groups <-- it is not annotation, but it could be modified by developer, if JMS will be installed (and i fixed SonataPageSite entity right now - i forgot to remove annotation there)

kunicmarko20 commented 6 years ago

Hm, I guess it can work that way, I am not happy tbh. We should maybe add a message after recipe/pack about that if we go this way.

I would rather have .xml|.yml config and they do w/e they want with it, if they do not have JMS it won't be imported because there is nothing looking for them.

kunicmarko20 commented 6 years ago

cc @sonata-project/contributors, wdyt?

OskarStark commented 6 years ago

Replace egeloen/ckeditor-bundle with other, sonata-project/SonataFormatterBundle#331

is merged, but do we need to wait for a releaase?

covex-nn commented 6 years ago

@OskarStark egeloen/ckeditor-bundle is not replaced, actualy. FormatterBundle still require it =(

rande commented 6 years ago

@sonata-project/contributors done! You now have 4 new repositories!

covex-nn commented 6 years ago

New version of symfony-cmf/routing-bundle with Symfony4 support was released and tested with https://github.com/sonata-project/SonataPageBundle/pull/981, so now it is possible to created a recipe for SonataPageBundle! I am planning to create a PR in symfong/recipes-contrib after https://github.com/sonata-project/SonataPageBundle/pull/996 will be merged and released

VincentLanglet commented 3 years ago

Closing as the missing library are abandoned