Skip to content

Allow Symfony 4.x as valid Symfony version #213

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

Merged
merged 5 commits into from
Oct 24, 2017

Conversation

pascaldevink
Copy link
Contributor

@pascaldevink pascaldevink commented Oct 19, 2017

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Documented? no
License MIT

Since Symfony released the first beta for version 4, which is not all that different from version 3.4, this composer change will allow it to work with that version (if the minimum-stability is set to beta at least).

Copy link
Contributor

@renatomefi renatomefi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool improvements 👍
Good to know SF 4 is becoming a reality for the bundle!

I have a few considerations, nothing too much!

@@ -42,7 +44,7 @@ install: composer update --prefer-source --no-interaction --optimize-autoloader

script:
- bin/phpunit --debug $( if [ $TEST_COVERAGE = true ]; then echo "-d xdebug.max_nesting_level=1000 --coverage-clover=build/logs/clover.xml"; fi; )
- if [ ${TRAVIS_PHP_VERSION} == "7.0" ]; then bin/php-cs-fixer fix --diff --dry-run -v; fi;
- if [ ${TRAVIS_PHP_VERSION} == "7.0" ]; then composer require --dev 'friendsofphp/php-cs-fixer:^2.0' && bin/php-cs-fixer fix --diff --dry-run -v; fi;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Let's make it run on 7.1 instead for speed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove it from requirements because it seems to not be compatible with Symfony 4

.travis.yml Outdated
- php: nightly
env: COMPOSER_UPDATE_FLAGS=--ignore-platform-reqs
env: SYMFONY_VERSION=4.0.* DEPENDENCIES=dev COMPOSER_UPDATE_FLAGS=--ignore-platform-reqs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make a build 7.1 with symfony 4 and leave the nightly more accordingly to our composer.json?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I will make the change

@@ -7,7 +7,7 @@ overblog_graphql:
class_namespace: "Overblog\\GraphQLBundle\\Exception\\__DEFINITIONS__"
exceptions:
errors:
- "InvalidArgumentException"
- 'InvalidArgumentException'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Feels like we are now mixing single and double quotes, maybe we can choose one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope no necessary, forget to rollback this

@mcg-web mcg-web force-pushed the symfony_4 branch 2 times, most recently from 3068be7 to a4df733 Compare October 20, 2017 11:58
@@ -16,5 +16,6 @@ Giving some love to PHP CS
---------------------------

```bash
bin/php-cs-fixer fix ./
composer require --dev 'friendsofphp/php-cs-fixer:^2.0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcg-web how about creating a composer script to install and run the php cs?

composer cs-fix

Could even do a composer test as well for the phpunit call

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against, we will dedicate a PR to this

@mcg-web
Copy link
Contributor

mcg-web commented Oct 20, 2017

@pascaldevink it seems that this is asking some more work to be ready to merge, I'm working on this thank for initializing the work 👍

@renatomefi
Copy link
Contributor

renatomefi commented Oct 20, 2017

Yeah, I've tried it myself a couple of weeks ago when making the SF Flex thingy, there's indeed some work necessary!

Maybe this is a 0.11.0 thing?

@pascaldevink
Copy link
Contributor Author

I would definitely argue that this requires a new version, this seems to be more than a patch, if only because php 5.5 is no longer supported.

It seems all the tests that fail are for hhvm and php 5.6 with symfony 3.1, is there anything I can do to help fix these?

@renatomefi renatomefi added this to the v0.11 milestone Oct 20, 2017
@renatomefi renatomefi added the wip label Oct 20, 2017
@mcg-web
Copy link
Contributor

mcg-web commented Oct 23, 2017

The issue with this update is the change of container cache compilation as now each service are become a php file against methods in older versions... This new feature hardly touching the way the tests works right now. Some reverse engineering is necessary to understand this.

@mcg-web mcg-web removed the wip label Oct 24, 2017
@mcg-web mcg-web requested a review from barduck007 October 24, 2017 07:11
@mcg-web mcg-web merged commit 07ba996 into overblog:master Oct 24, 2017
@mcg-web mcg-web modified the milestones: v0.11, v0.10 Nov 10, 2017
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.

4 participants