Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for BlockBundle 4.x #522

Closed
wants to merge 9 commits into from

Conversation

wbloszyk
Copy link
Member

Subject

@core23
After check #488 I think we can add support for BlockBundle 4.x. Can u check it?

I am targeting this branch, because it is PATCH.

@wbloszyk
Copy link
Member Author

@core23 What do you think about it? Can we remove AbstractAdminBlockService?
It is not used in any place. You can find only method call like here:
https://github.com/sonata-project/SonataPageBundle/blob/master/src/Admin/BlockAdmin.php#L217-L238

@sonata-project/contribiutors
If we can do it like BC, then major upgrade for other sonata-project bundles will be much simpler.

* @deprecated since sonata-project/block-bundle 3.16, to be removed in version 4.0.
*/
public function buildEditForm(FormMapper $form, BlockInterface $block)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{
{
$this->buildEditForm($formMapper, $block);

Copy link
Member Author

@wbloszyk wbloszyk Mar 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is AbstractAdminBlockService method, we should not change them. Self reference method make infinity loop.

@core23
Copy link
Member

core23 commented Mar 31, 2020

Looking at Travis, sonata-block 4 is never resolved

@wbloszyk
Copy link
Member Author

@core23
I think other dependencies should be update first (like MediaBundle). What do you think about this changes? Should I work on this?

@core23
Copy link
Member

core23 commented Mar 31, 2020

@core23
I think other dependencies should be update first (like MediaBundle). What do you think about this changes? Should I work on this?

As said several time on many different issues. We had some major changes on the old core bundle and the block bundle. This changes can only be resolved with new major versions on our projects.

The main goal is still a new admin bundle major version. After that we can try to find a way for all other projects.

@wbloszyk wbloszyk closed this Apr 23, 2020
@wbloszyk wbloszyk reopened this Apr 23, 2020
@wbloszyk wbloszyk marked this pull request as draft April 23, 2020 20:37
@SonataCI
Copy link
Collaborator

SonataCI commented Jun 2, 2020

Could you please rebase your PR and fix merge conflicts?

@github-actions
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 29, 2020
@@ -23,8 +23,8 @@
"require": {
"php": "^7.1",
"cocur/slugify": "^2.0 || ^3.0 || ^4.0",
"sonata-project/admin-bundle": "^3.31",
"sonata-project/core-bundle": "^3.14",
"sonata-project/admin-bundle": "^3.31 || dev-master",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not add this to the stable branch

@github-actions github-actions bot closed this Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants