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 one more dependency in composer for creating symfony-recipe #463

Closed
wants to merge 1 commit into from

Conversation

kricha
Copy link

@kricha kricha commented Jan 8, 2018

I am targeting this branch, because we are creating recipe for 3.x version.

Fix for symfony/recipes-contrib#235

Changelog

### Added
- added symfony/templating dependency to composer for using symfony recipes

@OskarStark
Copy link
Member

What about #457 ?

@kricha
Copy link
Author

kricha commented Jan 8, 2018

@OskarStark aha.. ok, i could close this one.

@jordisala1991
Copy link
Member

It could get merged independently from the other PR...

@kricha
Copy link
Author

kricha commented Jan 8, 2018

@OskarStark if all is ok, you can merge this one 😅

@jordisala1991
Copy link
Member

Please explain why did you added this dependency to composer on the changelog

@kricha
Copy link
Author

kricha commented Jan 8, 2018

@jordisala1991 because without this i'm getting this error:

Executing script cache:clear [KO]
 [KO]
Script cache:clear returned with error code 1
!!
!!  In FrameworkExtension.php line 217:
!!
!!    Templating support cannot be enabled as the Templating component is not ins
!!    talled.
!!
!!
!!

@jordisala1991
Copy link
Member

Yep, I get it, but you need to explain it on the changelog :) End users should know what does change with your PR

@kricha
Copy link
Author

kricha commented Jan 8, 2018

@jordisala1991 is it ok now?

@covex-nn
Copy link
Contributor

covex-nn commented Jan 8, 2018

This PR will not help to create recipe. Because, you cannot configure framework in your recipe, you cannot add this to your config (this is against rules):

framework:
    templating:
        engines: ["twig"]

So, even if you will add symfony/templating to composer.json, templating service will not be created, and you will see new error message about it

@kricha
Copy link
Author

kricha commented Jan 8, 2018

@covex-nn yes, but we can add something like that:

framework:                 # Remove this section
    templating:            # if you'll be using this in config/packages/framework.yaml
        engines: ['twig']  #

@covex-nn
Copy link
Contributor

covex-nn commented Jan 8, 2018

@aLkRicha these lines will not be accepted with comments or without them. See discussion about session symfony/recipes#262. Session can be enabled only with this config:

framework:
    session: ~

... but they say, that we cannot do this. If session cannot be activated in recipe config, then templating cannot be activated too. Also, see stof's comment about templating: symfony/flex#247 (comment). He said, that "they precisely want to discourage the usage of the Templating wrapper in Flex projects".

@kricha
Copy link
Author

kricha commented Jan 8, 2018

@covex-nn so there is no way to create sonataadmin recipe because of session?

@covex-nn
Copy link
Contributor

covex-nn commented Jan 8, 2018

See symfony/recipes#333 - session will be activated by default after merge. Right now there will be error about session service

@kricha
Copy link
Author

kricha commented Jan 8, 2018

We need #457 instead of this.

@kricha kricha closed this Jan 8, 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.

4 participants