configure GuzzleParamConverter implementation based on the SensioFramewo...

[xabbuh]

...rkExtraBundle version

This pull requests fixes #36. I'll add some tests later.

[xabbuh]

fixed the tests

[bendavies]

+1 @thewilkybarkid

[thewilkybarkid]

Looking good, the Travis config will need to be changed to something like:

env:
  - SYMFONY_VERSION="2.2.0" GUZZLE_VERSION="3.0.0" SENSIO_FRAMEWORK_EXTRA_VERSION="~2.2"
  - SYMFONY_VERSION="2.2.0" GUZZLE_VERSION="~3.1" SENSIO_FRAMEWORK_EXTRA_VERSION="~2.2"
  - SYMFONY_VERSION="~2.4"  GUZZLE_VERSION="3.0.0" SENSIO_FRAMEWORK_EXTRA_VERSION="~3.0"
  - SYMFONY_VERSION="~2.4"  GUZZLE_VERSION="~3.1" SENSIO_FRAMEWORK_EXTRA_VERSION="~3.0"

before_script:
  - composer require symfony/framework-bundle:${SYMFONY_VERSION} --no-update
  - composer require guzzle/guzzle:${GUZZLE_VERSION} --no-update
  - composer require sensio/framework-extra-bundle:${SENSIO_FRAMEWORK_EXTRA_VERSION} --dev --no-update
  - composer install --dev --prefer-source

as it's not testing 3.x at the moment.

It does throw a ReflectionException if the SensioFrameworkExtraBundle isn't installed, it'll need to be wrapped inside an interface_exists() check (the parameter will need to be set either way). It would be handy to have a single Travis run without the SensioFrameworkExtraBundle installed, but I'm not sure how we could do that.

I think the class should be renamed from GuzzleParamConverter2 to something like LegacyGuzzleParamConverter (or maybe both to GuzzleParamConverter3x and GuzzleParamConverter2x). Both class docs should also have something like "This version is compatible with SensioFrameworkExtraBundle (3|2).x." This should make it a little clearer what's going on!

[xabbuh]

Looking good, the Travis config will need to be changed to something like:

env:
  - SYMFONY_VERSION="2.2.0" GUZZLE_VERSION="3.0.0" > SENSIO_FRAMEWORK_EXTRA_VERSION="~2.2"
  - SYMFONY_VERSION="2.2.0" GUZZLE_VERSION="~3.1" SENSIO_FRAMEWORK_EXTRA_VERSION="~2.2"
  - SYMFONY_VERSION="~2.4"  GUZZLE_VERSION="3.0.0" SENSIO_FRAMEWORK_EXTRA_VERSION="~3.0"
  - SYMFONY_VERSION="~2.4"  GUZZLE_VERSION="~3.1" SENSIO_FRAMEWORK_EXTRA_VERSION="~3.0"

before_script:
  - composer require symfony/framework-bundle:${SYMFONY_VERSION} --no-update
  - composer require guzzle/guzzle:${GUZZLE_VERSION} --no-update
  - composer require sensio/framework-extra-bundle:${SENSIO_FRAMEWORK_EXTRA_VERSION} --dev --no-update
  - composer install --dev --prefer-source

as it's not testing 3.x at the moment.

Done. Is there a specific reason to test Symfony 2.2.0 instead of ~2.3? Furthermore, wouldn't it be good to include a test for Symfony 2.3 as well?

It does throw a ReflectionException if the SensioFrameworkExtraBundle isn't installed, it'll need to be wrapped inside an interface_exists() check (the parameter will need to be set either way). It would be handy to have a single Travis run without the SensioFrameworkExtraBundle installed, but I'm not sure how we could do that.

I moved the service definition into a separate XML file. So the service should only be registered when SensioFrameworkExtraBundle is installed.

I think the class should be renamed from GuzzleParamConverter2 to something like LegacyGuzzleParamConverter (or maybe both to GuzzleParamConverter3x and GuzzleParamConverter2x). Both class docs should also have something like "This version is compatible with SensioFrameworkExtraBundle (3|2).x." This should make it a little clearer what's going on!

Changed the names to GuzzleParamConverter2x and GuzzleParamConverter3x and added the API doc comment.

[thewilkybarkid]

Thanks. Could you rebase the changes? I'll then merge and tag a new release.

Is there a specific reason to test Symfony 2.2.0 instead of ~2.3? Furthermore, wouldn't it be good to include a test for Symfony 2.3 as well?

I would like to keep 2.2 compatibility (it's not EOL yet). There could be a 2.3.x test, but as long as it works with 2.2.x it's not really necessary.

[xabbuh]

@thewilkybarkid rebased on master.

I would like to keep 2.2 compatibility (it's not EOL yet). There could be a 2.3.x test, but as long as it works with 2.2.x it's not really necessary.

I added environment definitions for Symfony 2.3. But I can remove them if you don't agree.

[thewilkybarkid]

~2.3 means anything before 3.0-dev, so it'll be using 2.4.x at the moment. It would need to be 2.3.x.

[xabbuh]

Changed it.

[thewilkybarkid]

:thumbsup: Great, thanks for doing this!