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 Symfony 3.x to dependencies alongside 2.x #67

Merged
merged 3 commits into from Jan 13, 2016
Merged

Add Symfony 3.x to dependencies alongside 2.x #67

merged 3 commits into from Jan 13, 2016

Conversation

vveselinov
Copy link
Contributor

No description provided.

@igaponov
Copy link

You should add symfony=3.0 env to .travis.yml, so the maintainers will see that the build is passed.

@barryvdh
Copy link
Member

Looks good, can you add the env settings to the travis config, as @igaponov suggested?

@vveselinov
Copy link
Contributor Author

Done. However, Symfony 3.0 requires PHP >= 5.5.9, so some builds will fail.

@barryvdh
Copy link
Member

Okay we should probably tweak the build to just test fewer builds
e.g just 5.3 with lowest version constraints and test 1 version with both 2.x and 3.x

@vveselinov
Copy link
Contributor Author

Ok. I excluded the builds that have mismatched requirements. Travis config's a bit bloated now, but I don't know a better way to do it.

@barryvdh
Copy link
Member

Maybe a Travis expert like @GrahamCampbell could give a suggest? ;)

@barryvdh
Copy link
Member

Perhaps something like this would be enough, to test lowest on php 5.3 and 5.5 (so 2.1 and 3.0 respectively), and stable on all others.

language: php

php:
  - 5.3
  - 5.4
  - 5.5
  - 5.6
  - 7.0
  - hhvm

env:
  global:
    - setup=stable

matrix:
  include:
    - php: 5.3
      env: setup=lowest
    - php: 5.5
      env: setup=lowest

sudo: false

install:
  - if [[ $setup = 'stable' ]]; then travis_retry composer update --prefer-source --no-interaction --prefer-stable; fi
  - if [[ $setup = 'lowest' ]]; then travis_retry composer update --prefer-source --no-interaction --prefer-lowest --prefer-stable; fi

@barryvdh
Copy link
Member

I'm gonna merge this for now. We'll look into Travis optimisations later.

barryvdh added a commit that referenced this pull request Jan 13, 2016
Add Symfony 3.x to dependencies alongside 2.x
@barryvdh barryvdh merged commit 91d0db0 into thephpleague:master Jan 13, 2016
@barryvdh
Copy link
Member

Okay, so this doesn't work, because we require Guzzle 3, which requires Symfony 2.
guzzle/guzzle v3.9.3 requires symfony/event-dispatcher ~2.1

@barryvdh
Copy link
Member

Reverting for confusion.

@vveselinov
Copy link
Contributor Author

So ... how about branching a new major version that targets guzzle 6.x and both symfony 2.x and 3.x? I can prepare a pull request in a few days.

@barryvdh
Copy link
Member

See the roadmap: thephpleague/omnipay#274

We should probably create a PSR-7 version, without relying on Guzzle or Symfony.

But this will mean that every gateways needs to be updated also..

@lazychaser
Copy link

So no symfony 3.0 support in near future? I can't use this package with Laravel 5.2.

@barryvdh
Copy link
Member

Only if guzzle3 upgrades their Symfony dependancy.

@lazychaser
Copy link

Is there a problem using guzze 6?

@barryvdh
Copy link
Member

Yes, all gateways are bound to he Guzzle 3 Http client interface and request/response object.
So it's just a lot of work to convert the core + all gateways. But it should be done..

@delatbabel
Copy link
Contributor

Guzzle 3 is getting pretty old now. Sounds like what we need is a multi-gateway guzzle hackathon weekend :)

@barryvdh
Copy link
Member

Yes we should probably have seen this one coming a year ago..

@lazychaser
Copy link

Is it possible to write some kind of adapter to guzzle 6 that will not require gateways to be rewritten?

@delatbabel
Copy link
Contributor

Define what changes need to be made.
Call for volunteers.
Set up a weekend.
JFDI.

No, the adapter approach would probably not work. Either that or it would be more work than just converting the gateways. Everything's changed in Guzzle 6, even the package name is different. So all gateways, test cases, etc, would need to be changed.

@barryvdh
Copy link
Member

No, not realistically. The HttpClient is an interface, but the request/response objects are not.

See thephpleague/omnipay#320 for the ideas of moving to PSR-7 instead of Guzzle.

@delatbabel
Copy link
Contributor

Yes I read the PSR-7 thing. It's a better long term solution but a lot more work in the short term. So for the time being I think we need a guzzle update and look at the PSR-7 solution later.

@barryvdh
Copy link
Member

Guzzle 6 is basically PSR-7. It uses PSR-7 as request/response objects.

@delatbabel
Copy link
Contributor

I'm pinging a few people who know a bit more about guzzle than I do. Let's see if we can come up with some kind of estimate for the work that needs doing. A lot of gateways use guzzle in different ways, in particular there is at least one that calls curl directly. So what we do has to work for everyone.

@judgej
Copy link
Member

judgej commented Jan 15, 2016

The beauty of PSR-7 is that it is a lowest-common-denominator. OmniPay could provide request/response interfaces and and a client, all of which could extend PSR-7. That would mean each gateway would always have the low-level functions to be able to mess around directly with headers, body content etc. as well as being given higher-level functions to keep some of the message construction complexity out of the gateway drivers. Sitting behind that would be connectors for Guzzle 3/6/whatever.

That's how I would see it working. The message implementations and client that OmniPay provides could look pretty much the same as the Guzzle 6 versions, which are also PSR-7+Guzzle helpful features.

@leesiongchan
Copy link

Any progress on this?

@delatbabel
Copy link
Contributor

This is a v3.0 change, we are doing a lot of commits on v3.0 at the moment but don't yet have a release date.

@clytemnestra
Copy link

Any ETA? Can't use SF 3 with this.

@barryvdh
Copy link
Member

barryvdh commented Jun 2, 2016

Not yet, I think mostly the PSR-7 stuff needs some work (see thephpleague/omnipay#362 ), but rest is mostly there I think.

@barryvdh
Copy link
Member

barryvdh commented Jul 7, 2016

FYI, this has been sort of re-reverted. So we now do allow HttpFoundation 3.x. That is only a partial fix, because the Guzzle3 problem has not been solved.
It does however fix the use-case, for when you are just using some Symfony 3 components, but not the full Symfony 3 framework.
This should work for Silex or Laravel, as they just use other components directly (which allow 2.8 and 3.x).

You should however force the install of event-dispatcher 2.8, to downgrade your installed version, as I've described in the readme now..

@janvernieuwe
Copy link

How is the status of this?
This is currently blocking our upgrade to SF 3 and guzzle upgrade to v6 (for caching)

@tchapi
Copy link

tchapi commented Nov 28, 2016

Any status update on this one ? Quite blocking us too for upgrading to SF3 ...

@delatbabel
Copy link
Contributor

It's underway.

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.

None yet

10 participants