Update build to use stages (adding PHPCS to check for CS violations) #2849
Conversation
lcobucci
force-pushed
the
update-build
branch
from
e59fec1 to
7135769
Compare
.travis.yml
Outdated
| - if [[ "$MYSQL_VERSION" == "5.6" || "$MYSQL_VERSION" == "5.7" ]]; then mysql -e "CREATE SCHEMA doctrine_tests; GRANT ALL PRIVILEGES ON doctrine_tests.* to travis@'%'"; fi; | ||
| - if [[ "$MYSQL_VERSION" == "5.6" || "$MYSQL_VERSION" == "5.7" ]]; then mysql -e "CREATE SCHEMA test_create_database; GRANT ALL PRIVILEGES ON test_create_database.* to travis@'%'"; fi; | ||
| - if [[ "$MYSQL_VERSION" == "5.6" || "$MYSQL_VERSION" == "5.7" ]]; then mysql -e "CREATE SCHEMA test_drop_database; GRANT ALL PRIVILEGES ON test_drop_database.* to travis@'%'"; fi; | ||
| - if [[ "$MYSQL_VERSION" == "5.6" || "$MYSQL_VERSION" == "5.7" || "$MARIADB_VERSION" == "10.2" || "$MARIADB_VERSION" == "10.1" || "$MARIADB_VERSION" == "10.0" || "$DB" == "mysql" || "$DB" == "mysqli" ]]; then bash ./tests/travis/create-mysql-schema.sh; fi; |
There was a problem hiding this comment.
Consider using a multiline syntax here
tests/travis/create-mysql-schema.sh
Outdated
|
|
||
| mysql -e "CREATE SCHEMA doctrine_tests; GRANT ALL PRIVILEGES ON doctrine_tests.* to travis@'%'" | ||
| mysql -e "CREATE SCHEMA test_create_database; GRANT ALL PRIVILEGES ON test_create_database.* to travis@'%'"; | ||
| mysql -e "CREATE SCHEMA test_drop_database; GRANT ALL PRIVILEGES ON test_drop_database.* to travis@'%'"; |
There was a problem hiding this comment.
This is mostly sql. How about creating an sql file containing these 3 queries and executing in just one mysql call?
| "symfony/console": "2.*||^3.0" | ||
| "symfony/console": "2.*||^3.0", | ||
| "doctrine/coding-standard": "^1.0", | ||
| "squizlabs/php_codesniffer": "^3.0" |
There was a problem hiding this comment.
consider adding sort-packages: true to the config section, it would make diffs like this one less confusing.
|
Sorry, this is a no-go. We have 109 open PRs that would require rework if this goes through, therefore a CS change PR cannot happen. |
|
I don't really understand why this PR was closed. Build stages still should happen and migration to Trusty is a must. While argument with open PRs is relevant, CS changes are only part of this PR and imho could be split to separate PR targetting develop. Also CS could go to allowed failures. |
|
Also, there are tools can could help the 109 PRs rework, like StyleCI. But this PR should probably be split into several easier-to-review, easier-to-merge PRs |
|
The issue is with rebasing existing PRs: don't drop this burden on contributors |
|
They still have to rebase to make their builds pass on Travis. And with CS changes targeting develop, it should be fine, there are only few PRs (if any) targeting develop. |
|
@Majkl578 this patch was about |
|
@Majkl578 no we don't closing and reopening is enough in this case. I think travis tests a merge, not the last commit itself |
|
@Ocramius I'll remove the commits regarding the fixes, allow PHPCS to fail, and reopen this PR. |
lcobucci
force-pushed
the
update-build
branch
4 times, most recently
from
d9301ca to
21831c9
Compare
lcobucci
force-pushed
the
update-build
branch
from
21831c9 to
e5f3e9a
Compare
This is now mandatory on the new Travis-CI containers.
Modifying Travis-CI configuration to use build stages and removing explicit MySQL 5.6 execution (it's default now). PHPCS is running with PHP nightly version so we can have it failing for now.
lcobucci
force-pushed
the
update-build
branch
from
e5f3e9a to
a39ea6b
Compare
|
@Ocramius you can review this again now 😉 |
Since travis is now using Ubuntu Trusty as default (as explained in #2837) we need to create the schema manually in order to execute the tests. I've fixed it (applying basically the same solution that was done on #2835, #2838 and #2825), however I've also changed things to use the build stages (and added PHPCS to ensure that we follow our standards).
It's important to say that I've simplified the build jobs since MySQL 5.6 is the default version on Trusty.