Skip to content
This repository has been archived by the owner on Jul 22, 2022. It is now read-only.

Use render() function instead of block #375

Merged
merged 2 commits into from Jun 1, 2016

Conversation

wouterj
Copy link
Contributor

@wouterj wouterj commented Jun 1, 2016

Another step towards Symfony 3 compatibility.

@soullivaneuh
Copy link
Member

@wouterj Is that compatible with SF 2.3?

@wouterj
Copy link
Contributor Author

wouterj commented Jun 1, 2016

@@ -17,7 +17,7 @@ file that was distributed with this source code.
<h3 class="box-title">{{ 'content_tree' | trans({}, 'SonataDoctrinePHPCRAdmin') }}</h3>
</div>
<div class="box-body">
{% render controller('sonata.admin.doctrine_phpcr.tree_controller:treeAction', { 'root': settings.id, 'selected': settings.selected }) %}
{{ render(controller('sonata.admin.doctrine_phpcr.tree_controller:treeAction', { 'root': settings.id, 'selected': settings.selected })) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you consider also linebreaking this in a separate commit while you're at it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for doing that, another step towards sane code.

@wouterj
Copy link
Contributor Author

wouterj commented Jun 1, 2016

Tests fail because this bundle's requirements no longer support Symfony 2.3. What about dropping it? (Symfony 2.3 has reached end of maintainance some days ago)

@greg0ire
Copy link
Contributor

greg0ire commented Jun 1, 2016

Or maybe find a way to make it BC? @soullivaneuh to the rescue ? BTW, if this was planned to be BC, should it have been based on master? Why is master the default branch for this bundle ? It not very clear what the status is even after reading sonata-project/SonataAdminBundle#3731

@wouterj
Copy link
Contributor Author

wouterj commented Jun 1, 2016

@greg0ire it's about the Resource(Rest)Bundle dependency. This bundle's 1.0 version is going to be included in the CMF 2 release, which targets Symfony 2.8+ and PHP 5.5+.

As this bundle depends on it, this bundle's 2.0 version (master is 2.0 dev) will also only support 2.8+ and PHP 5.5+.

@greg0ire greg0ire removed the RTM label Jun 1, 2016
@greg0ire
Copy link
Contributor

greg0ire commented Jun 1, 2016

I think it is safe to drop 2.3 . ping @dbu @lsmith77

@wouterj
Copy link
Contributor Author

wouterj commented Jun 1, 2016

Btw, this is not an issue caused by this PR, master is broken by default. Can we please discuss this in another pull request and merge this one?

@greg0ire
Copy link
Contributor

greg0ire commented Jun 1, 2016

The build is green for master : https://travis-ci.org/sonata-project/SonataDoctrinePhpcrAdminBundle/builds/129346090 , so I'm very reluctant to merge this. It would mean we would have to check every job of every build to make sure the only tests that are failing are the one that fail in this PR. Besides, I think dropping 2.3 support can be done quite quickly.

@greg0ire
Copy link
Contributor

greg0ire commented Jun 1, 2016

Oh sorry I misread your post, master should indeed be broken.

@greg0ire
Copy link
Contributor

greg0ire commented Jun 1, 2016

Well since there is no current way to make it green, I guess I'll just merge.

@greg0ire greg0ire merged commit 62ab6e0 into sonata-project:master Jun 1, 2016
@greg0ire
Copy link
Contributor

greg0ire commented Jun 1, 2016

@soullivaneuh , in this file, does the ~ mean phpcr does not use a dev-kit-generated composer.json yet ? If yes are we supposed to drop support by doing a direct PR, or should be take that as an occasion to fill in the blanks in dev-kit ?

@wouterj wouterj deleted the fix_render_call branch June 1, 2016 10:41
@wouterj
Copy link
Contributor Author

wouterj commented Jun 1, 2016

Thanks for the merge!

@greg0ire
Copy link
Contributor

greg0ire commented Jun 1, 2016

Thanks for the merge!

Thanks for the contribution ;)

@soullivaneuh
Copy link
Member

does the ~ mean phpcr does not use a dev-kit-generated composer.json

First of all, dev-kit does not generate composer.json. It distpatch a lint.

This bundle is one of the projects I still have to integrate.

And yes, all projects with no configuration (~) are project not integrated with dev-kit for the files distpatcher.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants