sampart / BreadcrumbsBundle

A small breadcrumbs bundle for Symfony
Other
188 stars 68 forks source link

Added support for Symfony 3.4. #84

Closed modywzm closed 6 years ago

modywzm commented 6 years ago

Probably for Symfony 4 too.

bocharsky-bw commented 6 years ago

Looks like a duplicate of #82

sampart commented 6 years ago

I'm wondering if it's worth keeping both as #82 is more complex since it'll support Symfony 4 too. What do you think, @bocharsky-bw?

Also, @modywzm can you tell us more about what testing you've done here?

bocharsky-bw commented 6 years ago

If we merge it before #82, we'll have some merge conflicts in #82, but probably not a bit deal to fix them. Anyway, since #82 is a bit slow, I'm 👍 for merging this one first if it's ready

sampart commented 6 years ago

Great, thanks. Let's see what @modywzm comes back with about testing etc to see what's needed before we can merge here.

pculka commented 6 years ago

can you please merge it? Templating can be set with

framework:
    templating: { engines: [twig] }

in config/packages/framework.yaml for symfony 4

sampart commented 6 years ago

Hi @pculka. This needs testing before we can merge it. I was hoping @modywzm could confirm what testing has already been done here as it may have already been tested.

If you're happy to test this instead, I could then merge it - unfortunately, I don't have time for testing at this end.

What I'd ideally like is some very basic testing with the following Symfony versions:

Unfortunately we don't have automated test for this sort of thing yet - I've opened https://github.com/whiteoctober/BreadcrumbsBundle/issues/86 for that.

althaus commented 6 years ago

@sampart This PR only sets the services to be public instead of private. This cannot hurt any Symfony version (even 2.3) and stops the current deprecation messages. A proper solution is discussed in #82.

sampart commented 6 years ago

@althaus you're right. Sorry, I'd not had time to think through the implications of this change. I'll merge this as-is, then (FYI @modywzm @pculka). Thanks to everyone for your input and sorry for the delay here.

pculka commented 6 years ago

/yipi thank you! Now on to the other PR which helps resolve a few more issues :)

sampart commented 6 years ago

And here's a release which includes this: https://github.com/whiteoctober/BreadcrumbsBundle/releases/tag/1.3.0

althaus commented 6 years ago

@sampart No prob for me. I know that those new Symfony version can be a pain in the a** for third party bundles. So thanks for taking action and for the new release. 😀

sampart commented 6 years ago

thanks, @althaus :smiley: