Split output for HEAD & for BODY (fixes #8)

xyNNN / GoogleTagManagerBundle

Google Tag Manager Bundle for Symfony 2

https://github.com/xyNNN/GoogleTagManagerBundle

GNU Lesser General Public License v3.0

27 stars 15 forks source link

Split output for HEAD & for BODY (fixes #8) #10

Closed stefandoorn closed 7 years ago

stefandoorn commented 7 years ago

[*] Splitted the twig helpers for HEAD & BODY, while keeping it backwards compatible so both get introduced together if still using the old helper (added deprecation on it).
[*] Moved Extension to Twig folder, as Symfony docs suggest.
[*] Added interface on Helper
[*] Split views in sub views to load whatever appropriate but also easy to load all together
[*] General improvements

@xyNNN I think it's ready for review. As far as I can see, it's still without BC breaks.

stefandoorn commented 7 years ago

Not sure you got the notification, so ping @xyNNN :)

xyNNN commented 7 years ago

Currently I'm thinking about to remove the backwards compatibility and create a new major release. Then we also can drop the support for symfony 2.8 and perhaps increase the minimum PHP version up to 5.6 or even 7.x ?! 🤔

stefandoorn commented 7 years ago

Makes sense to me, as PHP 5.6 is EOL anyway and PHP 7 is the future. I think we can go that in another PR, not to make things too big.

We might be able to finish the things that still can work and are good on PHP 5.6 and draft a final 2.x release with some of the changes already added, and then go for a new major release (3.x) that only supports PHP 7. WDYT? In that case we still polish things a bit and have the proper head/body implementation as Google suggests in their documentation.

xyNNN commented 7 years ago

I think you're right and I'm fine with this changes :) I only saw some problems (wrong array notation and this bundle defines php 5.3 as requirement) but this wasn't introduced by this pull request :) So let's merge it :)