-
Notifications
You must be signed in to change notification settings - Fork 53
Test against symfony versions #90
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
Changes from 9 commits
bae14a7
1cb572b
f1cb956
2f59b40
092d832
37d3698
7920c09
f066efb
14df844
8a5122a
566ad7f
24ed526
5d04ded
2a71c9b
88594a1
901d830
73d4ede
921ddba
518f86b
e9e9073
8d1403b
bceddc0
8c62d22
dcb2bca
9d5c846
dc97863
03ca9ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,14 +8,6 @@ cache: | |
directories: | ||
- $HOME/.composer/cache/files | ||
|
||
php: | ||
- 5.4 | ||
- 5.5 | ||
- 5.6 | ||
- 7.0 | ||
- 7.1 | ||
- hhvm | ||
|
||
env: | ||
global: | ||
- TEST_COMMAND="composer test" | ||
|
@@ -27,11 +19,33 @@ branches: | |
matrix: | ||
fast_finish: true | ||
include: | ||
# Minimum supported PHP and Symfony version | ||
- php: 5.4 | ||
env: COMPOSER_FLAGS="--prefer-stable --prefer-lowest" COVERAGE=true TEST_COMMAND="composer test-ci" | ||
|
||
env: DEPENDENCIES="minimun" COVERAGE=true TEST_COMMAND="composer test-ci" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minimum |
||
|
||
# Test the latest stable release | ||
- php: 7.1 | ||
|
||
# Test LTS versions | ||
- php: 7.1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Food for thought: test the LTS releases against the lowest common version they and this package supports. I'd say that people on LTS releases are less likely to always run on the latest PHP version, so testing against a lower version might be more comparable to real-world usage. |
||
env: DEPENDENCIES="symfony/lts:v2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clever use of the package, great discovery! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if some bundle authors actively only want to support 2 Symfony versions at one time? What's what we're talking about in FOSUserBundle FriendsOfSymfony/FOSUserBundle#2639 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can still install a mixed set of 2.7 and 2.8 components. What about using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I realize I was unclear! What I really mean was: in FOSUserBundle, they only want to support Symfony 3 and Symfony 4 (and then perhaps just do bug-fixes fro 2.7/2.8 branches). In that situation, I guess they would just remove this one line and change the minimum dependencies matrix to 5.5 (from 5.4)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove the Symfony 2 LTS job and bump your min requirement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @weaverryan they in FOSUserBundle refers to only one maintainer of the bundle actually. I had not even seen this discussion yet (it was hidden in the middle of the thousand notifications I received during the hackday) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. re requiring symfony/symfony: that hides problems if we miss depending on some symfony components. if we do that, we should require each component we use in the configured version instead. (which duplicates composer.json, so still not very nice) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We still have the "Test the latest stable release" job which does not alter the |
||
- php: 7.1 | ||
env: DEPENDENCIES="symfony/lts:v3" | ||
|
||
# Latest beta release | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest testing against dev rather than beta (especially if you allow them to fail):
|
||
- php: 7.1 | ||
env: DEPENDENCIES="beta" | ||
|
||
allow_failures: | ||
# Latest beta is allowed to fail. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrong comment |
||
- php: 7.1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to specify the PHP version here. It will avoid having to keep it in sync when changing the version used by |
||
env: DEPENDENCIES="beta" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the latest beta should not be allowed to fail. Probably we really should allow it to fail, but I'm afraid people will not "notice" the failure. I'd rather push them to be a bit more proactive by noticing it :) |
||
|
||
before_install: | ||
- if [[ $COVERAGE != true ]]; then phpenv config-rm xdebug.ini || true; fi | ||
- if [ "$DEPENDENCIES" = "minimun" ]; then COMPOSER_FLAGS="--prefer-stable --prefer-lowest"; fi; | ||
- if [ "$DEPENDENCIES" = "beta" ]; then composer config minimum-stability beta; fi; | ||
- if [[ $DEPENDENCIES == *"/"* ]]; then composer require --no-update $DEPENDENCIES; fi; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and then just test for |
||
|
||
install: | ||
# To be removed when this issue will be resolved: https://github.com/composer/composer/issues/5355 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,7 +102,7 @@ private function isJson($stream) | |
|
||
json_decode($stream->getContents()); | ||
|
||
return json_last_error() == JSON_ERROR_NONE; | ||
return JSON_ERROR_NONE == json_last_error(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. strict comparison ? |
||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minimum supported PHP
is confusing here