Skip to content
This repository has been archived by the owner on Nov 25, 2020. It is now read-only.

Added support for Symfony 3.4. #84

Merged
merged 1 commit into from
Jan 9, 2018
Merged

Added support for Symfony 3.4. #84

merged 1 commit into from
Jan 9, 2018

Conversation

modywzm
Copy link
Contributor

@modywzm modywzm commented Dec 3, 2017

Probably for Symfony 4 too.

Added support for Symfony 3.4
@bocharsky-bw
Copy link
Contributor

Looks like a duplicate of #82

@sampart
Copy link
Owner

sampart commented Dec 14, 2017

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
Copy link
Contributor

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
Copy link
Owner

sampart commented Dec 14, 2017

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

@pculka
Copy link

pculka commented Jan 5, 2018

can you please merge it?
Templating can be set with

framework:
    templating: { engines: [twig] }

in config/packages/framework.yaml for symfony 4

@sampart
Copy link
Owner

sampart commented Jan 8, 2018

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:

  • 2.3 (yes, I know this is an unsupported Symfony version)
  • 2.7
  • 2.8
  • 3.0
  • 3.4 (obviously)

Unfortunately we don't have automated test for this sort of thing yet - I've opened #86 for that.

@althaus
Copy link

althaus commented Jan 9, 2018

@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
Copy link
Owner

sampart commented Jan 9, 2018

@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.

@sampart sampart merged commit db24185 into sampart:master Jan 9, 2018
@pculka
Copy link

pculka commented Jan 9, 2018

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

@sampart
Copy link
Owner

sampart commented Jan 9, 2018

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

@althaus
Copy link

althaus commented Jan 9, 2018

@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
Copy link
Owner

sampart commented Jan 9, 2018

thanks, @althaus 😃

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.

5 participants