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

Drop CoreBundle in Sonata 3 - process description #697

Closed wbloszyk closed 4 years ago

wbloszyk commented 4 years ago

CoreBundle use aliases in deprecated classes. This mean we can replace This class by new even in Sonata 3, becouse they equal and will be BC. For example: Sonata\CoreBundle\Validator\ErrorElement === Sonata\Form\Validator\ErrorElement. Other things like FormHelper::registerFormMapping can be optional - after checking is class exists.

This change will allow drop CoreBundle from dependencies and use:

TODO

wbloszyk commented 4 years ago

WDYT @core23 @phansys @VincentLanglet @greg0ire @contributors

  1. Should we drop all return type hints in 1.x-extension or only required? We should only drop return type declarations that prevent the migration.
  2. Should we move FormHelper to form-extensions? We should not move it.
greg0ire commented 4 years ago

Should we drop all return type hints in 1.x-extension or only required?

We should only drop return type declarations that prevent the migration IMO, if that is what you mean.

greg0ire commented 4 years ago

Should we move FormHelper to form-extensions?

I don't know what it does or why it wasn't migrated in the first place, but given the name that would make sense I suppose.

greg0ire commented 4 years ago

Why are all the PRs above with a draft status?

wbloszyk commented 4 years ago

@greg0ire becouse form-extensions and twig-extensions 0.1 stable are require.

wbloszyk commented 4 years ago

@core23 @greg0ire @phansys @VincentLanglet @contributors

I DONE this issue.

You can reproduce sandbox by:

git clone https://github.com/wbloszyk/sandbox.git
git checkout drop_core_preview
// create env.local with database config
vendor/bin/phing
wbloszyk commented 4 years ago

@contributors

Can you try check STEP 1 in this weekend? I need this change in next week. I will be greatful.

VincentLanglet commented 4 years ago

@contributors

You're pinging a random guy, instead of @sonata-project/contributors

wbloszyk commented 4 years ago

Lets try @sonata-project/contributors

wbloszyk commented 4 years ago

@greg0ire STEP 1 is almost done. Can you release this change (5 releases)? For now you can release CoreBundle. This will allow prepare STEP 2.

greg0ire commented 4 years ago

All done :)

wbloszyk commented 4 years ago

Thanks :)

wbloszyk commented 4 years ago

Some bundles still need update dependencies. Anyway PRs are independent. Feel free to check it.

wbloszyk commented 4 years ago

Based on: https://github.com/sonata-project/twig-extensions/issues/86#issuecomment-639601703

I will drop twig/twig-extension in favor for twig/extra-bundle and replace truncate to u.truncate. I will add DeprecatedTextExtensionTest for truncate based on this u.truncate filter to keep BC too.

jordisala1991 commented 4 years ago

I think that we only need to move the missing class.

wbloszyk commented 4 years ago

This will be enought. I think we can do something more. Anyway it is work for Monday. Weekend time start :).

wbloszyk commented 4 years ago

Probably drop CoreBundle in BlockBundle 3 is impossible. jms/di-extra-bundle do not allow upgrade it and also can not be drop without BC-break.

I think we can add support for BlockBundle 4 instead.

jordisala1991 commented 4 years ago

We are missing some bundles here like phpcr or mongodb also

wbloszyk commented 4 years ago

PageBundle functional test require SonataAdminBundle release. I will change to Ready for review after that.

jordisala1991 commented 4 years ago

Probably drop CoreBundle in BlockBundle 3 is impossible. jms/di-extra-bundle do not allow upgrade it and also can not be drop without BC-break.

https://github.com/schmittjoh/JMSDiExtraBundle/blob/master/composer.json

This bundle is not even compatible with sf4, and it is on our require dev. @sonata-project/contributors can it be an acceptable bc break if we remove support for sf 3 on block 3 and that forces us to remove that bu dle from require dev?

The problem is the admin 3 cant fully remove corebundle if block 3 does not remove core bundle, because it is a dependency.

VincentLanglet commented 4 years ago

@sonata-project/contributors can it be an acceptable bc break if we remove support for sf 3

Removing support for sf3 is not a BC-break.

jordisala1991 commented 4 years ago

I know, but removing support for sf3 will force us to remove jms di-extra-bundle. That’s why I am asking.

VincentLanglet commented 4 years ago

I know, but removing support for sf3 will force us to remove jms di-extra-bundle.

But since it's a dev requirement, it's not a BC-break either.

That's why I don't understand the following point

@sonata-project/contributors can it be an acceptable bc break if ...

Both removing support for SF3 and removing jms are not BC-break

If it's mandatory, let's go with it then.

wbloszyk commented 4 years ago

Force user to drop CoreBundle and extensions 0.x in Sonata 3 is mandatory, even it must be done.

wbloszyk commented 4 years ago

In extensions 1.x are conflicts:

    "conflict": {
        "sonata-project/core-bundle": ">=3.13"
    },

In CoreBundle 3.20:

    "require": {
        "sonata-project/form-extensions": "^0.1.1",
        "sonata-project/twig-extensions": "^0.1.1"
    }

Composer install CoreBundle 3.12 and extensions 1.x but should CoreBundle 3.20 and extensions 0.x.

I think we have to update extensions 1.x and set conflict to any CoreBundle version.

Remove process should look like:

CoreBundle with extensions 1.x should not be allowed.

@sonata-project/contributors WDYT?

greg0ire commented 4 years ago

It sounds like a good upgrade path :)

wbloszyk commented 4 years ago

@sonata-project/contributors I check amlost all Sonata 3 project. In some of it Symfony3 and php 7.1 is still support. I think we should drop it. WDYT?

core23 commented 4 years ago

@ sonata-project/contributors

This does not work for you, unless you are part of the contributors.

In some of it Symfony3 and php 7.1 is still support. I think we should drop it. WDYT?

We should update our BC promise in that case: https://github.com/sonata-project/dev-kit/blob/master/project/CONTRIBUTING.md#dependency-changes

wbloszyk commented 4 years ago

@core23 if I understood correctly php7.2 and Symfony 4.3 is minimum version which must be support.

jordisala1991 commented 4 years ago

We should update our BC promise in that case: https://github.com/sonata-project/dev-kit/blob/master/project/CONTRIBUTING.md#dependency-changes

Drop support for php 7.1 is allowed by the guidelines (it is not on the orange zone), and the sf 3 drop is allowed too (we give support for one lts already).

wbloszyk commented 4 years ago

@core23 @greg0ire @phansys @VincentLanglet @jordisala1991

I update test branch in sandbox. You can reproduce it by:

git clone https://github.com/wbloszyk/sandbox.git
git checkout drop_core_preview
// create env.local with database config
vendor/bin/phing
wbloszyk commented 4 years ago

@greg0ire Some release description for you.

Release process will be little big for it and contains drop corebundle and also truncate (twig/twig-extensions) in favor for u filter. Also there is some dependencies in tests which should be keep.

  1. AdminBundle
  2. Change truncate to u filter:
  3. Make CoreBundle optional in PageBundle
  4. Releases other bundles
  5. Merge to master branch
wbloszyk commented 4 years ago

@greg0ire Can you release FormatterBundle?

wbloszyk commented 4 years ago

@greg0ire Can you release:

This should fix functional test in PageBundle.

greg0ire commented 4 years ago

@wbloszyk please ask on the #releases channel on slack next time, it will be faster, I can't keep up with my github notifications :P

wbloszyk commented 4 years ago

@greg0ire I will be remember about it.

BTW. This issue will be done in this month (in June). :tada:

greg0ire commented 4 years ago

Thanks a lot for all this work! You rock!

jordisala1991 commented 4 years ago

This one is almost done, we are still missing the other persistence bundles, sandbox and remove it from dev kit, because most of the projects have special builds with corebundle

wbloszyk commented 4 years ago

I have sandbox for it but some bundles must be release before it. CoreBundle id now optional in sonata 3. I think we can keep this dev-kit config and must delete in sonata 4.

wbloszyk commented 4 years ago

Releases free from CoreBundle: 3.69 - admin-bundle 3.20 - block-bundle 3.12 - classification-bundle 3.2 - comment-bundle 3.19 - doctrine-orm-admin-bundle 3.3 - commerce 4.2 - formatter-bundle 3.25 - media-bundle 3.13 - news-bundle 3.8 - notification-bundle 3.18 - page-bundle 2.11 - seo-bundle 3.6 - timeline-bundle 4.6 - user-bundle 0.4 - dashboard-bundle

wbloszyk commented 4 years ago

What with SonataDoctrinePhpcrAdminBundle? After drop CoreBundle and bump dev-require `"jms/serializer-bundle": "^3.3", we little in conflict.

VincentLanglet commented 4 years ago

See https://github.com/sonata-project/dev-kit/issues/659

IMHO there is no active support on this bundle. Recently when we added a new feature, it was always on the two others storage bundle, with no backport on the Phpcr bundle since no maintainers are using it.

wbloszyk commented 4 years ago

I think we can close this issue. Feel free to open it if SonataDoctrinePhpcrAdminBundle will be support.