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 SonataBlockBundle #235

Closed
wants to merge 1 commit into from
Closed

add SonataBlockBundle #235

wants to merge 1 commit into from

Conversation

kricha
Copy link
Contributor

@kricha kricha commented Jan 5, 2018

Q A
License MIT

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

templating:
engines: ['twig']
sonata_block:
default_contexts: [cms]
Copy link
Contributor

Choose a reason for hiding this comment

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

cms context is never used anywhere, and it was renamed to sonata_page_bundle for SonataPageBundle (see https://stackoverflow.com/questions/22798670/what-is-context-in-sonata-block-bundle). So, i think cms should be removed from default_contexts here.

@OskarStark
Copy link
Contributor

//cc @greg0ire

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

@@ -0,0 +1 @@
sonata_block: {}
Copy link

Choose a reason for hiding this comment

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

"sonata_block" should be removed as it is empty

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

@@ -0,0 +1 @@
sonata_block: {}
Copy link

Choose a reason for hiding this comment

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

"sonata_block" should be removed as it is empty

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@kricha
Copy link
Contributor Author

kricha commented Jan 7, 2018

@OskarStark @greg0ire so, we need to add TwigBundle to SonataBlockBundle?

@greg0ire
Copy link
Contributor

greg0ire commented Jan 7, 2018

@kricha
Copy link
Contributor Author

kricha commented Jan 7, 2018

@greg0ire ok, i'll create pull-request

@OskarStark
Copy link
Contributor

OskarStark commented Jan 8, 2018

We released 3.92 of block-bundle

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

@kricha
Copy link
Contributor Author

kricha commented Jan 8, 2018

i think this will work after merging this #151

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

@covex-nn
Copy link
Contributor

covex-nn commented Jan 13, 2018

@aLkRicha please move config/packages/sonata_projects/sonata_block.yaml to config/packages directory. Travis will not be green, because of templating service. But there will not be a error message about default_contexts.

@OskarStark
Copy link
Contributor

I asked for a BlockBundle release @aLkRicha @covex-nn

@kricha
Copy link
Contributor Author

kricha commented Jan 16, 2018

@OskarStark @covex-nn i'm waiting this PR sonata-project/SonataBlockBundle#457

@covex-nn
Copy link
Contributor

@OskarStark @aLkRicha since 3.10.0 there is no need to create a recipe for BlockBundle:

  • Recipe is used only for new projects, it is used for initial configuration only

  • Projects with BlockBundle < v3.10 are already configured

  • New projects will use BlockBundle >= 3.10.0 with it's default configuration. See it for yourself:

    composer create-project symfony/skeleton .
    composer config extra.symfony.allow-contrib true
    composer require sonata-project/block-bundle

    There you will see error about templating service The service "sonata.block.service.container" has a dependency on a non-existent service "templating". And this dependency must be removed.

But i can be wrong: may be we should create recipe for 3.9.*, what do you think?

@covex-nn
Copy link
Contributor

@OskarStark @aLkRicha sonata-project/SonataBlockBundle#457 is merged and released with 3.11.0, but i'm still not sure if we need a recipe for BlockBundle

@Nyholm
Copy link
Member

Nyholm commented Jan 28, 2018

So, should I close this?

@jordisala1991
Copy link
Contributor

jordisala1991 commented Feb 5, 2018

Default context is no longer required, so maybe we don't need this recipe

@jordisala1991
Copy link
Contributor

And this one can be closed, it is not needed

@fabpot fabpot closed this Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants